Re: [Qemu-devel] [PATCH v13 07/13] Add debugging infrastructure

[Orit Wasserman]

On 06/27/2012 08:53 PM, Eric Blake wrote:
> On 06/27/2012 04:34 AM, Orit Wasserman wrote:
>> Signed-off-by: Orit Wasserman <address@hidden>
>> ---
>>  arch_init.c |   33 +++++++++++++++++++++++++++------
>>  1 files changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index 9dafb6e..ee20c33 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -44,6 +44,14 @@
>>  #include "exec-memory.h"
>>  #include "hw/pcspk.h"
>>  
>> +#ifdef DEBUG_ARCH_INIT
>> +#define DPRINTF(fmt, ...) \
>> +    do { fprintf(stdout, "arch_init: " fmt, ## __VA_ARGS__); } while (0)
> 
> Unfortunately, '## __VA_ARGS__' is a gcc extension not portable to C99.
> 
> But as HACKING recommends its use, you are not the first offender, nor
> should this patch clean it up (rather, it should be a global cleanup).
> 
> Here's a portable way to do it (borrowed from libvirt):
> 
>>> /* Helper macros to implement VIR_DEBUG using just C99.  This
>>>  * assumes you pass fewer than 15 arguments to VIR_DEBUG, but
>>>  * can easily be expanded if needed.
>>>  *
>>>  * Note that gcc provides extensions of "define a(b...) b" or
>>>  * "define a(b,...) b,##__VA_ARGS__" as a means of eliding a comma
>>>  * when no var-args are present, but we don't want to require gcc.
>>>  */
>>> #define VIR_ARG15(_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, 
>>> _14, _15, ...) _15
>>> #define VIR_HAS_COMMA(...) VIR_ARG15(__VA_ARGS__, 1, 1, 1, 1, 1, 1, 1, 1, 
>>> 1, 1, 1, 1, 1, 0)
>>>
>>> /* Form the name VIR_DEBUG_[01], then call that macro,
>>>  * according to how many arguments are present.  Two-phase due to
>>>  * macro expansion rules.  */
>>> #define VIR_DEBUG_EXPAND(a, b, ...)      \
>>>     VIR_DEBUG_PASTE(a, b, __VA_ARGS__)
>>> #define VIR_DEBUG_PASTE(a, b, ...)       \
>>>     a##b(__VA_ARGS__)
>>>
>>> /* Internal use only, when VIR_DEBUG has one argument.  */
>>> #define VIR_DEBUG_0(fmt)                 \
>>>     VIR_DOMAIN_DEBUG_1(fmt "%s", "")
>>>
>>> /* Internal use only, when VIR_DEBUG has >=two arguments.  */
>>> #define VIR_DEBUG_1(fmt, ...)            \
>>>     do { fprintf(stdout, "prefix: " fmt, __VA_ARGS__); } while (0)
>>>
>>> #define VIR_DEBUG(...)                           \
>>>     VIR_DEBUG_EXPAND(VIR_DEBUG_,          \
>>>                      VIR_HAS_COMMA(__VA_ARGS__), \
>>>                      __VA_ARGS__)
>>>
> 
> Ugly, huh?  Which is why I won't ask you to fix HACKING.
yes quite ugly :)
> 
> Note that if you live with the restriction that you always provide an
> additional argument beyond the format string, then life is much simpler
> for complying with C99:
> 
> #define DPRINTF(fmt, ...) \
>     do { fprintf(stdout, "prefix: " fmt, __VA_ARGS__); } while (0)
> 
> but it means you can't use DPRINTF("string") (you would instead have to
> use DPRINTF("string", "dummy"), which might trigger a gcc warning about
> printf argument mismatch, so to silence that warning, you would have to
> use DPRINTF("string%s", "") - which is basically what my goop from
> libvirt above was attempting to do automatically).
>