Fix #678: Fix stats print on ARM due to indeterminate state of "va_list" parameter after vasprintf

[IgnoreWarnings]

The state of "va_list" parameter is indeterminant after a call to vasprintf, which causes UB on ARM.
For detailed error explanation: #678.

1. Portability: Specifiying the "va" parameter as reference to ensure reproducable behavior on arm and x86.
2. "vstrcatf" now copies the va_list parameter to prevent it beeing undetermined after call to vasprintf.

@stv0g
I made the decision to put the responsibility of advancing the list in the called function rather than the caller because this is how the existing code in table.cpp:void Table::row(int count, ...) assumes it should work.

@fwege

[n-eiling]

LGTM, some minor comments below.

[n-eiling]

I would add null pointer checks for dest and fmt

[n-eiling]

I'm surprised you don't need const char* restrict fmt.

[stv0g]

To my knowledge restrict is a C not C++ keyword?

Are you concerned about aliasing between the *dest and fmt pointers?

I dont fully understand, why we might need it..

[n-eiling]

Not sure if this is real-time critical code. But you could avoid one allocation by doing:

size_t n = vsnprintf(NULL, 0, fmt, va_copy);
*dest = (char *)(realloc(*dest, n + i + 1));
vsnprintf(*dest + n, n+1, fmt, va_copy);

[stv0g]

I am wondering, isnt it also UB to call vsnprintf again with the same va_copy?

@IgnoreWarnings wrote in #678

This bug occures because "va_list" is used after beeing passed to "vasprintf", which leads to undeterministic behavior.
Functions like "vasprintf" "invoke va_arg at least once, the value of arg is indeterminate after the return." (https://en.cppreference.com/w/c/io/vfprintf : Notes).

[stv0g]

Hi @IgnoreWarnings,

I think a found a nicer solution to the issue in #935.

What do you think about it? Thanks for the analysis. I definitely learned something new :)

[IgnoreWarnings]

Hi @IgnoreWarnings,

I think a found a nicer solution to the issue in #935.

What do you think about it? Thanks for the analysis. I definitely learned something new :)

Feel free to solve this issue however you like.
I personally would have reimplemented without using va_lists at all with modern features like std::format instead of this whole manual allocation. But that would be quiet some work to do.

[IgnoreWarnings]

Moved to #935 .