Stefan Hajnoczi <stefa...@gmail.com> 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.