Fix incorrect va_start calls

[jepler]

The 2nd argument of va_start must always be the function parameter immediately before the "..." argument. Otherwise, the behavior is undefined.

Reproducer (compile with gcc -Wvarargs):

#include <stdarg.h>
#include <stdio.h>

void wrong_va_arg(const char *format, ...) {
    va_list ap;
    const char *newFormat = "whatever";
    (void)format;

    va_start(ap, newFormat);
    vprintf(newFormat, ap);
    va_end(ap);
}

godbolt link to reproducer: https://godbolt.org/z/YMoGKb9xh

[CLAassistant]

All committers have signed the CLA.

[caveman99]

Thanks for your contribution, but that change defeats the point of the original change. Namely to add an \n to the format string. The original code actually works while your code would just print all logging without a newline. Did you actually test the change or did you just go by the compiler warning?

[jepler]

This does not to my knowledge "fix a bug": I checked gcc arm and gcc x86_64 and both generate the same code. It's likely a modern compiler just ignores the 2nd argument of va_start in favor of the information in the function prototype.

[jepler]

I am changing the second argument of va_start, which must be the named argument just before the ... argument. Since the argmuent to log_to_serial remains newFormat, the revised, added-newline version of the format string is used when actual formatting is done.

        va_list arg;
        va_start(arg, format);

        log_to_serial(logLevel, newFormat, arg);
        log_to_syslog(logLevel, newFormat, arg);

Here's a example that shows that the revised code with va_start(arg, format) + vprintf(newFormat) works properly. Godbolt version.

#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int printf_with_newline(const char *format, ...) {
    // append \n to format
    size_t len = strlen(format);
    char *newFormat = new char[len + 2];
    strcpy(newFormat, format);
    newFormat[len] = '\n';
    newFormat[len + 1] = '\0';

    va_list arg;
    va_start(arg, format);            // <-- va_start argument must be the actual function argument 
    int r = vprintf(newFormat, arg);  // <-- argument to vprintf can differ
    va_end(arg);

    delete newFormat;
    return r;
}

int main() {
    printf_with_newline("Your random number is %d",
                        rand() % 6);
    printf_with_newline("this appears on its own line");
}

output:

Your random number is 1
this appears on its own line

[caveman99]

Sorry, i only reviewed that from a mobile device and thought you changed all of the occurances. I tested a few binaries now with the patch and logging is still fine.

[jepler]

Thank you for taking a second look, I appreciate it!