qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 01/16] x86: Convert conditional compilation o


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v2 01/16] x86: Convert conditional compilation of debug printfs to regular ifs
Date: Tue, 13 May 2014 09:15:54 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0

On 05/13/2014 01:02 AM, Marc Marí wrote:
> Modify debug macros to have the same format through the codebase and use 
> regular
> ifs instead of ifdef.
> 
> As the debug printf is always put in code, some casting had to be added to 
> avoid
> warnings treated as errors at compile time.

Umm, where in this patch did you add casting?  Don't add bogus lines to
the commit message (I was assuming that it might be a case where the
code has type mismatches, and where something like PRId32 would be
better than adding a cast - but couldn't find where that was. Maybe
other patches in the series are affected?)

> 
> Signed-off-by: Marc Marí <address@hidden>
> ---

> -#define DEBUG(fmt, ...)                                       \
> -    do {                                                      \
> -        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
> -    } while (0)

The old code was line-wrapped to fit in 80 columns...

> +#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 1
>  #else
> -#define DEBUG(fmt, ...)
> +#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 0
>  #endif
>  
> +#define DEBUG(fmt, ...) QEMU_DPRINTF(DEVICE_ASSIGNMENT_DEBUG_ENABLED, 
> "pci_assign", fmt, ## __VA_ARGS__)

the new code should probably try to do similar:

#define DEBUG(fmt, ...)                                         \
    QEMU_DPRINTF(DEVICE_ASSIGNMENT_DEBUG_ENABLED,               \
                 "pci_assign", fmt, ## __VA_ARGS__)

Although __VA_ARGS__ is required by C99, the use of ##__VA_ARGS__ is a
gcc extension; are you sure that all other supported compilers handle
it?  (I guess that's just clang)

If you want something portable to C99, just use one fewer macro
argument, so that you are guaranteed that __VA_ARGS__ will be non-empty
(that is, subsume fmt into the ...):

#define DEBUG(...)                                              \
    QEMU_DPRINTF(DEVICE_ASSIGNMENT_DEBUG_ENABLED,               \
                 "pci_assign", __VA_ARGS__)


>  
> +#define mb_debug(a...) QEMU_DPRINTF(DEBUG_MULTIBOOT_ENABLED, "i386 
> multiboot", a)

Use of 'a...' as a named var-arg parameter is a gcc extension, not
portable to C99.  Again, will this compile on non-gcc?  Unnamed ...
coupled with __VA_ARGS__ is the only fully portable way to do var-args.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org



Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]