Re: [Qemu-devel] [RFC][PATCH]Add timestamp to error message

[Laszlo Ersek]

On 02/01/13 15:53, Seiji Aguchi wrote:

>  /*
>   * Print an error message to current monitor if we have one, else to stderr.
>   * Format arguments like sprintf().  The result should not contain
> @@ -207,6 +296,7 @@ void error_report(const char *fmt, ...)
>  {
>      va_list ap;
>  
> +    error_print_timestamp();
>      error_print_loc();
>      va_start(ap, fmt);
>      error_vprintf(fmt, ap);
> -- 1.7.1

Side note: strictly in theory, this would result in two vfprintf()
calls. The message log would remain "record oriented", but (again, in
theory) another thread *might* get interleaved and mess up our format
with a parallel call to error_report() (or more deeply, to fprintf()).

Importantly I'm not talking about "corrupting data"; stdio streams are
automatically locked by the fprintf() family. The thing (theoretically,
possibly) corrupted would be our record-oriented message format, by
interleaved printfs.

(a) I'm not sure if this is possible at all in qemu.

(b) Anyway, there are two ways to fix it:

(b1) In error_report(), lock the stream across the two printfs with
flockfile(). Probably overkill, and in case we're printing to the
monitor, wasteful/useless. Or,

(b2) Format the full message (including the timestamp) into a buffer
(sprintf, vsprintf(), or their glib wrappers with automatic allocation,
if any) and print it with a single error_printf("%s", buf).

Anyway I absolutely do not insist on this, so sorry for the noise.

Thanks
Laszlo