Re: [Qemu-devel] [PATCH 05/43] windbg: added helper features

[Mihail Abakumov]

Eric Blake писал 2017-09-26 20:27:
> On 09/26/2017 12:13 PM, Alistair Francis wrote:
>
> > > +#if (WINDBG_DEBUG_ON)
> > > +
> > > +# define WINDBG_DEBUG(...) do {    \
> > > +    printf("Debug: " __VA_ARGS__); \
> > > +    printf("\n");                  \
> > > +} while (false)
> > > +
> > > +# define WINDBG_ERROR(...) do {    \
> > > +    printf("Error: " __VA_ARGS__); \
> > > +    printf("\n");                  \
> > > +} while (false)
> >
> > Use qemu_log() instead of printf().
> >
> > Have a look as some other files for the usual way we handle debug 
> > printing.
> >
> > > +
> > > +#else
> > > +
> > > +# define WINDBG_DEBUG(...)
> > > +# define WINDBG_ERROR(...) error_report(WINDBG ": " __VA_ARGS__)
>
> What's more - as written, your approach is prone to bit-rot: the
> debug/error statements are not compared against -Werror except for the
> rare person that enables debugging.  Better is go make the macro
> unconditionally expand to something that triggers -Wformat checking, 
> but
> guarded by an if(0) for normal use.  Or even switch to trace points
> rather than debugging statements, so that you can control at runtime 
> how
> much debugging information you want, rather than having to recompile to
> turn it on and off.

Thank you for your feedback.

I corrected it like this
#define WINDBG_DEBUG(...) do {             \
    if (WINDBG_DEBUG_ON) {                 \
        qemu_log(WINDBG ": " __VA_ARGS__); \
        qemu_log("\n");                    \
    }                                      \
} while (false)

You can find a new version here:
    
https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg03912.html

--
Thanks,
Mihail Abakumov