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

[Eric Blake]

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature