While looking into the code, I noticed that there is a function called lib_mvsprintf() into vice/src/lib.c.
It seems that this function must return a formatted string allocated from the heap and you used function vsnprintf() by testing continuously into a loop its return value, until it won't return that the print has been done correctly.
In my opinion, there is a better and simpler solution for implementing this task.
When function vsnprinf receives its "size" parameter to zero, it works as usual but it has the effect to ignore the pointer to the output buffer. So, when used in this manner, the function does basically the task to calculate the exact size of the output string and the returned value can be used to allocate the size you need. The body of the new function is much shorter and direct:
va_copy(args, ap);
maxlen = vsnprintf (NULL, 0, fmt, args);
if ((p = lib_malloc (maxlen + 1)) == NULL)
return NULL;
vsnprintf (p, maxlen + 1, fmt, ap);
This behavior of vsnprintf() is standard, so I think that it can be used without problems.
Sincerely.
The patch.
I haven't checked the patch for correctness yet, but please use brackets for each branch, ie change:
into:
I know it's a bit pedantic, but it'll save us from weird bugs in the future.
that patch is re-introducing another bug... missing va_end :)
applied a hopefully working adaption in r38771 - thanks for the hint, i should have done it this way in the first place in my previous fixing attempt :)
Ouch! It escaped to my attention, sorry! ^^;
That's right, also in my opinion those two lines are redundant. If you want to preserve "ap" parameter, "args" should be used also inside the second vsnprintf().
Thank you very much.
Sincerely.
Last edit: Carlo Bramini 2020-10-13
The second use of
va_copy()andva_end()is pointless (the line in between them doesn't useargs).oops