Re: [Qemu-devel] [PATCH v2 6/7] maint: Fix macros with broken 'do/while(0); ' usage

[Juan Quintela]

Eric Blake  wrote:
> The point of writing a macro embedded in a 'do { ... } while (0)'
> loop (particularly if the macro has multiple statements or would
> otherwise end with an 'if' statement) is so that the macro can be
> used as a drop-in statement with the caller supplying the
> trailing ';'.  Although our coding style frowns on brace-less 'if':
>   if (cond)
> statement;
>   else
> something else;
> that is the classic case where failure to use do/while(0) wrapping
> would cause the 'else' to pair with any embedded 'if' in the macro
> rather than the intended outer 'if'.  But conversely, if the macro
> includes an embedded ';', then the same brace-less coding style
> would now have two statements, making the 'else' a syntax error
> rather than pairing with the outer 'if'.  Thus, even though our
> coding style with required braces is not impacted, ending a macro
> with ';' makes our code harder to port to projects that use
> brace-less styles.
>
> The change should have no semantic impact.  I was not able to
> fully compile-test all of the changes (as some of them are
> examples of the ugly bit-rotting debug print statements that are
> completely elided by default, and I didn't want to recompile
> with the necessary -D witnesses - cleaning those up is left as a
> bite-sized task for another day); I did, however, audit that for
> all files touched, all callers of the changed macros DID supply
> a trailing ';' at the callsite, and did not appear to be used
> as part of a brace-less conditional.
>
> Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 
>
> Signed-off-by: Eric Blake 
> Acked-by: Cornelia Huck 
> Reviewed-by: Michael S. Tsirkin 
> Acked-by: Dr. David Alan Gilbert 

Reviewed-by: Juan Quintela 

[Qemu-devel] [PATCH v2 6/7] maint: Fix macros with broken 'do/while(0); ' usage

[Eric Blake]

The point of writing a macro embedded in a 'do { ... } while (0)'
loop (particularly if the macro has multiple statements or would
otherwise end with an 'if' statement) is so that the macro can be
used as a drop-in statement with the caller supplying the
trailing ';'.  Although our coding style frowns on brace-less 'if':
  if (cond)
statement;
  else
something else;
that is the classic case where failure to use do/while(0) wrapping
would cause the 'else' to pair with any embedded 'if' in the macro
rather than the intended outer 'if'.  But conversely, if the macro
includes an embedded ';', then the same brace-less coding style
would now have two statements, making the 'else' a syntax error
rather than pairing with the outer 'if'.  Thus, even though our
coding style with required braces is not impacted, ending a macro
with ';' makes our code harder to port to projects that use
brace-less styles.

The change should have no semantic impact.  I was not able to
fully compile-test all of the changes (as some of them are
examples of the ugly bit-rotting debug print statements that are
completely elided by default, and I didn't want to recompile
with the necessary -D witnesses - cleaning those up is left as a
bite-sized task for another day); I did, however, audit that for
all files touched, all callers of the changed macros DID supply
a trailing ';' at the callsite, and did not appear to be used
as part of a brace-less conditional.

Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 

Signed-off-by: Eric Blake 
Acked-by: Cornelia Huck 
Reviewed-by: Michael S. Tsirkin 
Acked-by: Dr. David Alan Gilbert 
---
 tests/acpi-utils.h | 8 
 ui/sdl_zoom_template.h | 8 
 audio/paaudio.c| 4 ++--
 hw/adc/stm32f2xx_adc.c | 2 +-
 hw/block/m25p80.c  | 2 +-
 hw/char/cadence_uart.c | 2 +-
 hw/char/stm32f2xx_usart.c  | 2 +-
 hw/display/cg3.c   | 2 +-
 hw/display/dpcd.c  | 2 +-
 hw/display/xlnx_dp.c   | 2 +-
 hw/dma/pl330.c | 2 +-
 hw/dma/xlnx-zynq-devcfg.c  | 2 +-
 hw/dma/xlnx_dpdma.c| 2 +-
 hw/i2c/i2c-ddc.c   | 2 +-
 hw/misc/auxbus.c   | 2 +-
 hw/misc/macio/mac_dbdma.c  | 4 ++--
 hw/misc/mmio_interface.c   | 2 +-
 hw/misc/stm32f2xx_syscfg.c | 2 +-
 hw/misc/zynq_slcr.c| 2 +-
 hw/net/cadence_gem.c   | 2 +-
 hw/ssi/mss-spi.c   | 2 +-
 hw/ssi/stm32f2xx_spi.c | 2 +-
 hw/ssi/xilinx_spi.c| 2 +-
 hw/ssi/xilinx_spips.c  | 2 +-
 hw/timer/a9gtimer.c| 2 +-
 hw/timer/cadence_ttc.c | 2 +-
 hw/timer/mss-timer.c   | 2 +-
 hw/timer/stm32f2xx_timer.c | 2 +-
 hw/tpm/tpm_passthrough.c   | 2 +-
 hw/tpm/tpm_tis.c   | 2 +-
 migration/rdma.c   | 2 +-
 target/arm/translate-a64.c | 2 +-
 target/s390x/kvm.c | 2 +-
 tests/tcg/test-mmap.c  | 2 +-
 34 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
index d5ca5b6238..ac52abd0dd 100644
--- a/tests/acpi-utils.h
+++ b/tests/acpi-utils.h
@@ -32,7 +32,7 @@ typedef struct {
 do {\
 memread(addr, , sizeof(field));   \
 addr += sizeof(field);  \
-} while (0);
+} while (0)

 #define ACPI_READ_ARRAY_PTR(arr, length, addr)  \
 do {\
@@ -40,7 +40,7 @@ typedef struct {
 for (idx = 0; idx < length; ++idx) {\
 ACPI_READ_FIELD(arr[idx], addr);\
 }   \
-} while (0);
+} while (0)

 #define ACPI_READ_ARRAY(arr, addr)   \
 ACPI_READ_ARRAY_PTR(arr, sizeof(arr) / sizeof(arr[0]), addr)
@@ -56,7 +56,7 @@ typedef struct {
 ACPI_READ_FIELD((table)->oem_revision, addr);\
 ACPI_READ_ARRAY((table)->asl_compiler_id, addr); \
 ACPI_READ_FIELD((table)->asl_compiler_revision, addr);   \
-} while (0);
+} while (0)

 #define ACPI_ASSERT_CMP(actual, expected) do { \
 char ACPI_ASSERT_CMP_str[5] = {}; \
@@ -77,7 +77,7 @@ typedef struct {
 ACPI_READ_FIELD((field).bit_offset, addr);   \
 ACPI_READ_FIELD((field).access_width, addr); \
 ACPI_READ_FIELD((field).address, addr);  \
-} while (0);
+} while (0)


 uint8_t acpi_calc_checksum(const uint8_t *data, int len);
diff --git a/ui/sdl_zoom_template.h b/ui/sdl_zoom_template.h
index 3bb508b51e..6a424adfb4 100644
--- a/ui/sdl_zoom_template.h
+++ b/ui/sdl_zoom_template.h
@@ -34,22 +34,22 @@
 #define setRed(r, pcolor) do { \
 *pcolor = ((*pcolor) & (~(dpf->Rmask))) + \
   (((r) & (dpf->Rmask >> dpf->Rshift)) << dpf->Rshift); \
-} while (0);
+} while (0)

 #define setGreen(g, pcolor) do { \
 *pcolor = ((*pcolor) & (~(dpf->Gmask))) + \
   (((g) & (dpf->Gmask >> dpf->Gshift)) << dpf->Gshift); \
-} while (0);
+}