Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus.

[Anthony Liguori]

Andreas Färber <address@hidden> writes:

> Am 26.11.2012 15:33, schrieb Anthony Liguori:
>> address@hidden writes:
>>> +#define DEBUG_VIRTIO_BUS 1
>>> +
>>> +#define DPRINTF(fmt, ...) if (DEBUG_VIRTIO_BUS) {                        \
>>> +                            printf("virtio_bus: " fmt , ## __VA_ARGS__); \
>>> +                          }
>> 
>> #ifdef DEBUG_VIRTIO_BUS
>> #define DPRINTF(fmt, ...) ...
>> #else
>> #define DPRINTF(fmt, ...) do { } while (0)
>> #endif
>> 
>> You're leaving a dangling if clause which can do very strange things if
>> used before an else statement.
>
> If you look at the change history, this was a change in a reaction to me
> pointing to a proposal by Samsung. I see your point with the else
> statement, that can be circumvented by adding else {}. The reason is to
> avoid DPRINTF()s bitrotting because your "do { } while (0)" performs no
> compile-tests on "fmt, ..." arguments.

This is a well-used idiom in QEMU.  We shouldn't try to change idioms in
random patch series.

If you want to change the way we do DPRINTF(), it should be done
globally in its own series.

Regards,

Anthony Liguori

>
> Andreas
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg