Re: [Qemu-devel] [PATCH] debug: made printf always compile in debug output

[Markus Armbruster]

Stefan Hajnoczi <address@hidden> writes:

> On Mon, Mar 27, 2017 at 02:50:47PM +0300, Danil Antonov wrote:
>> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
>> index 2b0f3e1..bf880c1 100644
>> --- a/hw/acpi/pcihp.c
>> +++ b/hw/acpi/pcihp.c
>> @@ -39,13 +39,16 @@
>>  #include "qom/qom-qobject.h"
>>  #include "qapi/qmp/qint.h"
>> 
>> -//#define DEBUG
>> 
>> -#ifdef DEBUG
>> -# define ACPI_PCIHP_DPRINTF(format, ...)     printf(format, ## __VA_ARGS__)
>> -#else
>> -# define ACPI_PCIHP_DPRINTF(format, ...)     do { } while (0)
>> -#endif
>> +#ifndef DEBUG
>> +#define DEBUG 0
>> +#endif
>> +
>> +#define ACPI_PCIHP_DPRINTF(fmt, ...) do {     \
>> +    if (DEBUG) {                              \
>> +        fprintf(stderr, fmt, ## __VA_ARGS__); \
>> +    }                                         \
>> +} while (0);
>
> The semicolon isn't necessary.  The caller provides it.

s/isn't necessary/is wrong/

>   ACPI_PCIHP_DPRINTF("foo\n");

Your defintion of ACPI_PCIHP_DPRINTF() expands this into *two*
statements, the one you want, plus a null statement:

    do { ... } while (0);;
    \___________________/|
      first statment     |
                     null statement

Breaks in places where C expects a statement, e.g.

    if (cond1)
        if (cond2)
            ACPI_PCIHP_DPRINTF(...);
        else
            bar();

Gets parsed like

    if (cond1) {
        if (cond2) {
            first statement
        }
    }
    null statement
    else // compiler chokes, fortunately
        bar();

> It's normal to define statement-like macros with do { ... } while (0)
> and no semicolon in the definition.

A macro should expand into exactly one expression or exactly one
statement.  Intentional deviations from this rule may be okay.
Accidental deviations aren't.