util.c: add xsyslog_ev()

[ksmurchison]

No description provided.

[elliefm]

Passing our ap directly to buf_vprintf() here like this is bad. buf_vprintf() passes it straight through to vsnprintf(), whose documentation says (emphasis mine):

The functions vprintf(), vfprintf(), vdprintf(), vsprintf(), vsnprintf() are equivalent to the functions printf(), fprintf(), dprintf(), sprintf(), snprintf(), respectively, except that they are called with a va_list instead of a variable number of arguments. These functions do not call the va_end macro. Because they invoke the va_arg macro, the value of ap is undefined after the call. See stdarg(3).

But we expect to loop around and keep extracting further arguments from ap afterwards.

We could take a copy of va and pass the copy to buf_vprintf(), but the arguments consumed by the format wouldn't be removed from the original, and I don't think we can tell how many it used in order to skip over them on the next loop. We could assume it used exactly one, which looks like the design intent, but the compiler can't enforce that.

As implemented, and disregarding the behaviour being undefined, I think something like xsyslog_ev(LOG_DEBUG, "some_event", "key1", "%i %i", value1a, value1b, "key2", "%i", value2); would kind of work (i.e. it's not key/format/value triples, but key/(format/format-args) pairs with the format-args sub lists flattened). If we explicitly consumed exactly one value instead, that call would fall apart -- value1b would be the next key, "key2" would be the next format, "%i" would be ignored by buf_vprintf but stepped over, "value2" would be another key, and now we've run out of args but are still trying to read them, if we somehow haven't already crashed earlier.

We've also lost the ability for -Wformat-security to tell us when we accidentally misuse it, since there's no longer exactly one format string argument in a known position. Actually, with a strict enough security warning level, I think this might introduce complaints about using a format argument that's not a string literal. That's not always possible to avoid, but it is always a funny smell.

[ksmurchison]

@elliefm I have rewritten this to pass a copy of the current va_list to buf_printf() and then skip over the args that were consumed by parsing the format string

[elliefm]

Oof, that skip-over code is more complicated than I had thought it would be, but looking at it, it does indeed need to be that complicated, because we need to know the types for all the va_arg() calls! But at this point we're basically writing our own printf format string parser, so we've long since lost any advantage we might have gotten from using printf format strings...