[Qemu-devel] [PATCH 0/3] macro do/while (0) cleanup

[Qemu-devel] [PATCH 0/3] macro do/while (0) cleanup

[Eric Blake]

Noticed this by chance in the tests/ directory, so I broadened
it to a grep of the entire code base.  I suspect many of the
bad macros were the victims of copy-and-paste from some other
bad location (particularly given how many bit-rotten debug
print macros were involved).

https://wiki.qemu.org/BiteSizedTasks#Bitrot_prevention is still
left for someone else, for another day.

Eric Blake (3):
  net: Drop unusual use of do { } while (0);
  mips: Tweak location of ';' in macros
  maint: Fix macros with broken 'do/while(0);' usage

 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/net/pcnet.c             | 20 ++++++++++----------
 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/mips/msa_helper.c   | 34 ++++++++++++++++++----------------
 target/s390x/kvm.c         |  2 +-
 tests/tcg/test-mmap.c      |  2 +-
 36 files changed, 70 insertions(+), 68 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 1/3] net: Drop unusual use of do { } while (0);

[Eric Blake]

For a couple of macros in pcnet.c, we have to provide a new scope
to avoid compiler warnings about declarations in the middle of a
switch statement that aren't in a sub-scope.  But use of
'do { ... } while (0);' merely to provide that new scope is arcane
overkill, compared to just using '{ ... }'.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 hw/net/pcnet.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 654455355f..641bf2fd88 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -455,32 +455,32 @@ static inline void pcnet_rmd_store(PCNetState *s, struct pcnet_RMD *rmd,
 #define CHECK_RMD(ADDR,RES) do {                \
     switch (BCR_SWSTYLE(s)) {                   \
     case 0x00:                                  \
-        do {                                    \
+        {                                       \
             uint16_t rda[4];                    \
             s->phys_mem_read(s->dma_opaque, (ADDR), \
                 (void *)&rda[0], sizeof(rda), 0); \
             (RES) |= (rda[2] & 0xf000)!=0xf000; \
             (RES) |= (rda[3] & 0xf000)!=0x0000; \
-        } while (0);                            \
+        }                                       \
         break;                                  \
     case 0x01:                                  \
     case 0x02:                                  \
-        do {                                    \
+        {                                       \
             uint32_t rda[4];                    \
             s->phys_mem_read(s->dma_opaque, (ADDR), \
                 (void *)&rda[0], sizeof(rda), 0); \
             (RES) |= (rda[1] & 0x0000f000L)!=0x0000f000L; \
             (RES) |= (rda[2] & 0x0000f000L)!=0x00000000L; \
-        } while (0);                            \
+        }                                       \
         break;                                  \
     case 0x03:                                  \
-        do {                                    \
+        {                                       \
             uint32_t rda[4];                    \
             s->phys_mem_read(s->dma_opaque, (ADDR), \
                 (void *)&rda[0], sizeof(rda), 0); \
             (RES) |= (rda[0] & 0x0000f000L)!=0x00000000L; \
             (RES) |= (rda[1] & 0x0000f000L)!=0x0000f000L; \
-        } while (0);                            \
+        }                                       \
         break;                                  \
     }                                           \
 } while (0)
@@ -488,22 +488,22 @@ static inline void pcnet_rmd_store(PCNetState *s, struct pcnet_RMD *rmd,
 #define CHECK_TMD(ADDR,RES) do {                \
     switch (BCR_SWSTYLE(s)) {                   \
     case 0x00:                                  \
-        do {                                    \
+        {                                       \
             uint16_t xda[4];                    \
             s->phys_mem_read(s->dma_opaque, (ADDR), \
                 (void *)&xda[0], sizeof(xda), 0); \
             (RES) |= (xda[2] & 0xf000)!=0xf000; \
-        } while (0);                            \
+        }                                       \
         break;                                  \
     case 0x01:                                  \
     case 0x02:                                  \
     case 0x03:                                  \
-        do {                                    \
+        {                                       \
             uint32_t xda[4];                    \
             s->phys_mem_read(s->dma_opaque, (ADDR), \
                 (void *)&xda[0], sizeof(xda), 0); \
             (RES) |= (xda[1] & 0x0000f000L)!=0x0000f000L; \
-        } while (0);                            \
+        }                                       \
         break;                                  \
     }                                           \
 } while (0)
-- 
2.14.3

Re: [Qemu-devel] [PATCH 1/3] net: Drop unusual use of do { } while (0);

[Thomas Huth]

On 30.11.2017 14:41, Eric Blake wrote:
> For a couple of macros in pcnet.c, we have to provide a new scope
> to avoid compiler warnings about declarations in the middle of a
> switch statement that aren't in a sub-scope.  But use of
> 'do { ... } while (0);' merely to provide that new scope is arcane
> overkill, compared to just using '{ ... }'.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  hw/net/pcnet.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[Qemu-devel] [PATCH 2/3] mips: Tweak location of ';' in macros

[Eric Blake]

It is more typical to provide the ';' by the caller of a macro
than to embed it in the macro itself; this is because syntax
highlight engines can get confused if a macro is called without
a semicolon before the closing '}'.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 target/mips/msa_helper.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
index f167a42655..8fb7a369ca 100644
--- a/target/mips/msa_helper.c
+++ b/target/mips/msa_helper.c
@@ -682,13 +682,13 @@ static inline int64_t msa_mod_u_df(uint32_t df, int64_t arg1, int64_t arg2)
     do {                                \
         e = SIGNED_EVEN(a, df);         \
         o = SIGNED_ODD(a, df);          \
-    } while (0);
+    } while (0)

 #define UNSIGNED_EXTRACT(e, o, a, df)   \
     do {                                \
         e = UNSIGNED_EVEN(a, df);       \
         o = UNSIGNED_ODD(a, df);        \
-    } while (0);
+    } while (0)

 static inline int64_t msa_dotp_s_df(uint32_t df, int64_t arg1, int64_t arg2)
 {
@@ -1120,9 +1120,11 @@ void helper_msa_splat_df(CPUMIPSState *env, uint32_t df, uint32_t wd,
 #define MSA_LOOP_COND_D MSA_LOOP_COND(DF_DOUBLE)

 #define MSA_LOOP(DF) \
+    do { \
         for (i = 0; i < (MSA_LOOP_COND_ ## DF) ; i++) { \
-            MSA_DO_ ## DF \
-        }
+            MSA_DO_ ## DF; \
+        } \
+    } while (0)

 #define MSA_FN_DF(FUNC)                                             \
 void helper_msa_##FUNC(CPUMIPSState *env, uint32_t df, uint32_t wd, \
@@ -1135,17 +1137,17 @@ void helper_msa_##FUNC(CPUMIPSState *env, uint32_t df, uint32_t wd, \
     uint32_t i;                                                     \
     switch (df) {                                                   \
     case DF_BYTE:                                                   \
-        MSA_LOOP_B                                                  \
+        MSA_LOOP_B;                                                 \
         break;                                                      \
     case DF_HALF:                                                   \
-        MSA_LOOP_H                                                  \
+        MSA_LOOP_H;                                                 \
         break;                                                      \
     case DF_WORD:                                                   \
-        MSA_LOOP_W                                                  \
+        MSA_LOOP_W;                                                 \
         break;                                                      \
     case DF_DOUBLE:                                                 \
-        MSA_LOOP_D                                                  \
-       break;                                                       \
+        MSA_LOOP_D;                                                 \
+        break;                                                      \
     default:                                                        \
         assert(0);                                                  \
     }                                                               \
@@ -1168,7 +1170,7 @@ void helper_msa_##FUNC(CPUMIPSState *env, uint32_t df, uint32_t wd, \
     do {                                \
         R##DF(pwx, i) = pwt->DF[2*i];   \
         L##DF(pwx, i) = pws->DF[2*i];   \
-    } while (0);
+    } while (0)
 MSA_FN_DF(pckev_df)
 #undef MSA_DO

@@ -1176,7 +1178,7 @@ MSA_FN_DF(pckev_df)
     do {                                \
         R##DF(pwx, i) = pwt->DF[2*i+1]; \
         L##DF(pwx, i) = pws->DF[2*i+1]; \
-    } while (0);
+    } while (0)
 MSA_FN_DF(pckod_df)
 #undef MSA_DO

@@ -1184,7 +1186,7 @@ MSA_FN_DF(pckod_df)
     do {                                \
         pwx->DF[2*i]   = L##DF(pwt, i); \
         pwx->DF[2*i+1] = L##DF(pws, i); \
-    } while (0);
+    } while (0)
 MSA_FN_DF(ilvl_df)
 #undef MSA_DO

@@ -1192,7 +1194,7 @@ MSA_FN_DF(ilvl_df)
     do {                                \
         pwx->DF[2*i]   = R##DF(pwt, i); \
         pwx->DF[2*i+1] = R##DF(pws, i); \
-    } while (0);
+    } while (0)
 MSA_FN_DF(ilvr_df)
 #undef MSA_DO

@@ -1200,7 +1202,7 @@ MSA_FN_DF(ilvr_df)
     do {                                \
         pwx->DF[2*i]   = pwt->DF[2*i];  \
         pwx->DF[2*i+1] = pws->DF[2*i];  \
-    } while (0);
+    } while (0)
 MSA_FN_DF(ilvev_df)
 #undef MSA_DO

@@ -1208,7 +1210,7 @@ MSA_FN_DF(ilvev_df)
     do {                                    \
         pwx->DF[2*i]   = pwt->DF[2*i+1];    \
         pwx->DF[2*i+1] = pws->DF[2*i+1];    \
-    } while (0);
+    } while (0)
 MSA_FN_DF(ilvod_df)
 #undef MSA_DO
 #undef MSA_LOOP_COND
@@ -1222,7 +1224,7 @@ MSA_FN_DF(ilvod_df)
         uint32_t k = (pwd->DF[i] & 0x3f) % (2 * n);                         \
         pwx->DF[i] =                                                        \
             (pwd->DF[i] & 0xc0) ? 0 : k < n ? pwt->DF[k] : pws->DF[k - n];  \
-    } while (0);
+    } while (0)
 MSA_FN_DF(vshf_df)
 #undef MSA_DO
 #undef MSA_LOOP_COND
-- 
2.14.3

Re: [Qemu-devel] [PATCH 2/3] mips: Tweak location of ';' in macros

[Philippe Mathieu-Daudé]

On 11/30/2017 10:41 AM, Eric Blake wrote:
> It is more typical to provide the ';' by the caller of a macro
> than to embed it in the macro itself; this is because syntax
> highlight engines can get confused if a macro is called without
> a semicolon before the closing '}'.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/mips/msa_helper.c | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
> index f167a42655..8fb7a369ca 100644
> --- a/target/mips/msa_helper.c
> +++ b/target/mips/msa_helper.c
> @@ -682,13 +682,13 @@ static inline int64_t msa_mod_u_df(uint32_t df, int64_t arg1, int64_t arg2)
>      do {                                \
>          e = SIGNED_EVEN(a, df);         \
>          o = SIGNED_ODD(a, df);          \
> -    } while (0);
> +    } while (0)
> 
>  #define UNSIGNED_EXTRACT(e, o, a, df)   \
>      do {                                \
>          e = UNSIGNED_EVEN(a, df);       \
>          o = UNSIGNED_ODD(a, df);        \
> -    } while (0);
> +    } while (0)
> 
>  static inline int64_t msa_dotp_s_df(uint32_t df, int64_t arg1, int64_t arg2)
>  {
> @@ -1120,9 +1120,11 @@ void helper_msa_splat_df(CPUMIPSState *env, uint32_t df, uint32_t wd,
>  #define MSA_LOOP_COND_D MSA_LOOP_COND(DF_DOUBLE)
> 
>  #define MSA_LOOP(DF) \
> +    do { \
>          for (i = 0; i < (MSA_LOOP_COND_ ## DF) ; i++) { \
> -            MSA_DO_ ## DF \
> -        }
> +            MSA_DO_ ## DF; \
> +        } \
> +    } while (0)
> 
>  #define MSA_FN_DF(FUNC)                                             \
>  void helper_msa_##FUNC(CPUMIPSState *env, uint32_t df, uint32_t wd, \
> @@ -1135,17 +1137,17 @@ void helper_msa_##FUNC(CPUMIPSState *env, uint32_t df, uint32_t wd, \
>      uint32_t i;                                                     \
>      switch (df) {                                                   \
>      case DF_BYTE:                                                   \
> -        MSA_LOOP_B                                                  \
> +        MSA_LOOP_B;                                                 \
>          break;                                                      \
>      case DF_HALF:                                                   \
> -        MSA_LOOP_H                                                  \
> +        MSA_LOOP_H;                                                 \
>          break;                                                      \
>      case DF_WORD:                                                   \
> -        MSA_LOOP_W                                                  \
> +        MSA_LOOP_W;                                                 \
>          break;                                                      \
>      case DF_DOUBLE:                                                 \
> -        MSA_LOOP_D                                                  \
> -       break;                                                       \
> +        MSA_LOOP_D;                                                 \
> +        break;                                                      \
>      default:                                                        \
>          assert(0);                                                  \
>      }                                                               \
> @@ -1168,7 +1170,7 @@ void helper_msa_##FUNC(CPUMIPSState *env, uint32_t df, uint32_t wd, \
>      do {                                \
>          R##DF(pwx, i) = pwt->DF[2*i];   \
>          L##DF(pwx, i) = pws->DF[2*i];   \
> -    } while (0);
> +    } while (0)
>  MSA_FN_DF(pckev_df)
>  #undef MSA_DO
> 
> @@ -1176,7 +1178,7 @@ MSA_FN_DF(pckev_df)
>      do {                                \
>          R##DF(pwx, i) = pwt->DF[2*i+1]; \
>          L##DF(pwx, i) = pws->DF[2*i+1]; \
> -    } while (0);
> +    } while (0)
>  MSA_FN_DF(pckod_df)
>  #undef MSA_DO
> 
> @@ -1184,7 +1186,7 @@ MSA_FN_DF(pckod_df)
>      do {                                \
>          pwx->DF[2*i]   = L##DF(pwt, i); \
>          pwx->DF[2*i+1] = L##DF(pws, i); \
> -    } while (0);
> +    } while (0)
>  MSA_FN_DF(ilvl_df)
>  #undef MSA_DO
> 
> @@ -1192,7 +1194,7 @@ MSA_FN_DF(ilvl_df)
>      do {                                \
>          pwx->DF[2*i]   = R##DF(pwt, i); \
>          pwx->DF[2*i+1] = R##DF(pws, i); \
> -    } while (0);
> +    } while (0)
>  MSA_FN_DF(ilvr_df)
>  #undef MSA_DO
> 
> @@ -1200,7 +1202,7 @@ MSA_FN_DF(ilvr_df)
>      do {                                \
>          pwx->DF[2*i]   = pwt->DF[2*i];  \
>          pwx->DF[2*i+1] = pws->DF[2*i];  \
> -    } while (0);
> +    } while (0)
>  MSA_FN_DF(ilvev_df)
>  #undef MSA_DO
> 
> @@ -1208,7 +1210,7 @@ MSA_FN_DF(ilvev_df)
>      do {                                    \
>          pwx->DF[2*i]   = pwt->DF[2*i+1];    \
>          pwx->DF[2*i+1] = pws->DF[2*i+1];    \
> -    } while (0);
> +    } while (0)
>  MSA_FN_DF(ilvod_df)
>  #undef MSA_DO
>  #undef MSA_LOOP_COND
> @@ -1222,7 +1224,7 @@ MSA_FN_DF(ilvod_df)
>          uint32_t k = (pwd->DF[i] & 0x3f) % (2 * n);                         \
>          pwx->DF[i] =                                                        \
>              (pwd->DF[i] & 0xc0) ? 0 : k < n ? pwt->DF[k] : pws->DF[k - n];  \
> -    } while (0);
> +    } while (0)
>  MSA_FN_DF(vshf_df)
>  #undef MSA_DO
>  #undef MSA_LOOP_COND
> 

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

[Eric Blake]

The point of writing a macro embedded in a 'do { ... } while (0)'
loop 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;
the use of do/while (0) in a macro is absolutely essential for the
purpose of avoiding a syntax error on the 'else' - but it only works
if there is no trailing ';' in the macro (as the ';' in the code
calling the macro would then be a second statement and cause the
'else' to not pair to the 'if').

Many of the places touched in this code are examples of the ugly
bit-rotting debug print statements; cleaning those up is left as
a bite-sized task for another day.

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

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 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, &field, 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);
+} while (0)

 #define setBlue(b, pcolor) do { \
     *pcolor = ((*pcolor) & (~(dpf->Bmask))) + \
               (((b) & (dpf->Bmask >> dpf->Bshift)) << dpf->Bshift); \
-} while (0);
+} while (0)

 #define setAlpha(a, pcolor) do { \
     *pcolor = ((*pcolor) & (~(dpf->Amask))) + \
               (((a) & (dpf->Amask >> dpf->Ashift)) << dpf->Ashift); \
-} while (0);
+} while (0)

 static void glue(sdl_zoom_rgb, BPP)(SDL_Surface *src, SDL_Surface *dst, int smooth,
                                    SDL_Rect *dst_rect)
diff --git a/audio/paaudio.c b/audio/paaudio.c
index 65beb6f010..2a35e6f82c 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -89,7 +89,7 @@ static inline int PA_STREAM_IS_GOOD(pa_stream_state_t x)
             }                                                   \
             goto label;                                         \
         }                                                       \
-    } while (0);
+    } while (0)

 #define CHECK_DEAD_GOTO(c, stream, rerror, label)                       \
     do {                                                                \
@@ -107,7 +107,7 @@ static inline int PA_STREAM_IS_GOOD(pa_stream_state_t x)
             }                                                           \
             goto label;                                                 \
         }                                                               \
-    } while (0);
+    } while (0)

 static int qpa_simple_read (PAVoiceIn *p, void *data, size_t length, int *rerror)
 {
diff --git a/hw/adc/stm32f2xx_adc.c b/hw/adc/stm32f2xx_adc.c
index 90fe9de299..13f31ad2f7 100644
--- a/hw/adc/stm32f2xx_adc.c
+++ b/hw/adc/stm32f2xx_adc.c
@@ -37,7 +37,7 @@
     if (STM_ADC_ERR_DEBUG >= lvl) { \
         qemu_log("%s: " fmt, __func__, ## args); \
     } \
-} while (0);
+} while (0)

 #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index a2438b9ed2..62b5221bde 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -40,7 +40,7 @@
         fprintf(stderr,  ": %s: ", __func__); \
         fprintf(stderr, ## __VA_ARGS__); \
     } \
-} while (0);
+} while (0)

 /* Fields for FlashPartInfo->flags */

diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index 6143494060..fbdbd463bb 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -33,7 +33,7 @@
 #define DB_PRINT(...) do { \
     fprintf(stderr,  ": %s: ", __func__); \
     fprintf(stderr, ## __VA_ARGS__); \
-    } while (0);
+    } while (0)
 #else
     #define DB_PRINT(...)
 #endif
diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c
index 268e435338..07b462d4b6 100644
--- a/hw/char/stm32f2xx_usart.c
+++ b/hw/char/stm32f2xx_usart.c
@@ -34,7 +34,7 @@
     if (STM_USART_ERR_DEBUG >= lvl) { \
         qemu_log("%s: " fmt, __func__, ## args); \
     } \
-} while (0);
+} while (0)

 #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)

diff --git a/hw/display/cg3.c b/hw/display/cg3.c
index e069c4484c..cafd9f47ef 100644
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -63,7 +63,7 @@
     if (DEBUG_CG3) { \
         printf("CG3: " fmt , ## __VA_ARGS__); \
     } \
-} while (0);
+} while (0)

 #define TYPE_CG3 "cgthree"
 #define CG3(obj) OBJECT_CHECK(CG3State, (obj), TYPE_CG3)
diff --git a/hw/display/dpcd.c b/hw/display/dpcd.c
index ce92ff6e2a..943002bee5 100644
--- a/hw/display/dpcd.c
+++ b/hw/display/dpcd.c
@@ -39,7 +39,7 @@
     if (DEBUG_DPCD) {                                                          \
         qemu_log("dpcd: " fmt, ## __VA_ARGS__);                                \
     }                                                                          \
-} while (0);
+} while (0)

 #define DPCD_READABLE_AREA                      0x600

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index 561f828e7a..ead4e1a0e4 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -34,7 +34,7 @@
     if (DEBUG_DP) {                                                            \
         qemu_log("xlnx_dp: " fmt , ## __VA_ARGS__);                            \
     }                                                                          \
-} while (0);
+} while (0)

 /*
  * Register offset for DP.
diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c
index 32cf8399b8..d071049233 100644
--- a/hw/dma/pl330.c
+++ b/hw/dma/pl330.c
@@ -29,7 +29,7 @@
     if (PL330_ERR_DEBUG >= lvl) {\
         fprintf(stderr, "PL330: %s:" fmt, __func__, ## args);\
     } \
-} while (0);
+} while (0)

 #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)

diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c
index 3b10523430..12bb2e3716 100644
--- a/hw/dma/xlnx-zynq-devcfg.c
+++ b/hw/dma/xlnx-zynq-devcfg.c
@@ -43,7 +43,7 @@
     if (XLNX_ZYNQ_DEVCFG_ERR_DEBUG) { \
         qemu_log("%s: " fmt, __func__, ## args); \
     } \
-} while (0);
+} while (0)

 REG32(CTRL, 0x00)
     FIELD(CTRL,     FORCE_RST,          31,  1) /* Not supported, wr ignored */
diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
index 8ceb21ddb3..077c7da9cc 100644
--- a/hw/dma/xlnx_dpdma.c
+++ b/hw/dma/xlnx_dpdma.c
@@ -34,7 +34,7 @@
     if (DEBUG_DPDMA) {                                                         \
         qemu_log("xlnx_dpdma: " fmt , ## __VA_ARGS__);                         \
     }                                                                          \
-} while (0);
+} while (0)

 /*
  * Registers offset for DPDMA.
diff --git a/hw/i2c/i2c-ddc.c b/hw/i2c/i2c-ddc.c
index 6b92e95c73..199dac9e41 100644
--- a/hw/i2c/i2c-ddc.c
+++ b/hw/i2c/i2c-ddc.c
@@ -30,7 +30,7 @@
     if (DEBUG_I2CDDC) {                                                        \
         qemu_log("i2c-ddc: " fmt , ## __VA_ARGS__);                            \
     }                                                                          \
-} while (0);
+} while (0)

 /* Structure defining a monitor's characteristics in a
  * readable format: this should be passed to build_edid_blob()
diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c
index 1182745044..b4cacd664b 100644
--- a/hw/misc/auxbus.c
+++ b/hw/misc/auxbus.c
@@ -40,7 +40,7 @@
     if (DEBUG_AUX) {                                                           \
         qemu_log("aux: " fmt , ## __VA_ARGS__);                                \
     }                                                                          \
-} while (0);
+} while (0)

 #define TYPE_AUXTOI2C "aux-to-i2c-bridge"
 #define AUXTOI2C(obj) OBJECT_CHECK(AUXTOI2CState, (obj), TYPE_AUXTOI2C)
diff --git a/hw/misc/macio/mac_dbdma.c b/hw/misc/macio/mac_dbdma.c
index 0eddf2e700..1b2a69b3ef 100644
--- a/hw/misc/macio/mac_dbdma.c
+++ b/hw/misc/macio/mac_dbdma.c
@@ -52,7 +52,7 @@
     if (DEBUG_DBDMA) { \
         printf("DBDMA: " fmt , ## __VA_ARGS__); \
     } \
-} while (0);
+} while (0)

 #define DBDMA_DPRINTFCH(ch, fmt, ...) do { \
     if (DEBUG_DBDMA) { \
@@ -60,7 +60,7 @@
             printf("DBDMA[%02x]: " fmt , (ch)->channel, ## __VA_ARGS__); \
         } \
     } \
-} while (0);
+} while (0)

 /*
  */
diff --git a/hw/misc/mmio_interface.c b/hw/misc/mmio_interface.c
index 894e9801cb..3b0e2039a3 100644
--- a/hw/misc/mmio_interface.c
+++ b/hw/misc/mmio_interface.c
@@ -39,7 +39,7 @@ static uint64_t mmio_interface_counter;
     if (DEBUG_MMIO_INTERFACE) {                                                \
         qemu_log("mmio_interface: 0x%" PRIX64 ": " fmt, s->id, ## __VA_ARGS__);\
     }                                                                          \
-} while (0);
+} while (0)

 static void mmio_interface_init(Object *obj)
 {
diff --git a/hw/misc/stm32f2xx_syscfg.c b/hw/misc/stm32f2xx_syscfg.c
index 7c45833d09..7f10195862 100644
--- a/hw/misc/stm32f2xx_syscfg.c
+++ b/hw/misc/stm32f2xx_syscfg.c
@@ -34,7 +34,7 @@
     if (STM_SYSCFG_ERR_DEBUG >= lvl) { \
         qemu_log("%s: " fmt, __func__, ## args); \
     } \
-} while (0);
+} while (0)

 #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)

diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index 44304d48be..d6bdd027ef 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -30,7 +30,7 @@
             fprintf(stderr,  ": %s: ", __func__); \
             fprintf(stderr, ## __VA_ARGS__); \
         } \
-    } while (0);
+    } while (0)

 #define XILINX_LOCK_KEY 0x767b
 #define XILINX_UNLOCK_KEY 0xdf0d
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 3943187572..0fa4b0dc44 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -34,7 +34,7 @@
 #define DB_PRINT(...) do { \
     fprintf(stderr,  ": %s: ", __func__); \
     fprintf(stderr, ## __VA_ARGS__); \
-    } while (0);
+    } while (0)
 #else
     #define DB_PRINT(...)
 #endif
diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
index d60daba882..185e1a3920 100644
--- a/hw/ssi/mss-spi.c
+++ b/hw/ssi/mss-spi.c
@@ -35,7 +35,7 @@
     if (MSS_SPI_ERR_DEBUG >= lvl) { \
         qemu_log("%s: " fmt "\n", __func__, ## args); \
     } \
-} while (0);
+} while (0)

 #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)

diff --git a/hw/ssi/stm32f2xx_spi.c b/hw/ssi/stm32f2xx_spi.c
index 26a1b4ddf5..69514da9fb 100644
--- a/hw/ssi/stm32f2xx_spi.c
+++ b/hw/ssi/stm32f2xx_spi.c
@@ -35,7 +35,7 @@
     if (STM_SPI_ERR_DEBUG >= lvl) { \
         qemu_log("%s: " fmt, __func__, ## args); \
     } \
-} while (0);
+} while (0)

 #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)

diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
index 33482f04de..83585bc8b2 100644
--- a/hw/ssi/xilinx_spi.c
+++ b/hw/ssi/xilinx_spi.c
@@ -36,7 +36,7 @@
 #define DB_PRINT(...) do { \
     fprintf(stderr,  ": %s: ", __func__); \
     fprintf(stderr, ## __VA_ARGS__); \
-    } while (0);
+    } while (0)
 #else
     #define DB_PRINT(...)
 #endif
diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index ef56d35f2c..c070393dc9 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -43,7 +43,7 @@
         fprintf(stderr,  ": %s: ", __func__); \
         fprintf(stderr, ## __VA_ARGS__); \
     } \
-} while (0);
+} while (0)

 /* config register */
 #define R_CONFIG            (0x00 / 4)
diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c
index ce1dc63911..96d534d8a8 100644
--- a/hw/timer/a9gtimer.c
+++ b/hw/timer/a9gtimer.c
@@ -37,7 +37,7 @@
         fprintf(stderr,  ": %s: ", __func__); \
         fprintf(stderr, ## __VA_ARGS__); \
     } \
-} while (0);
+} while (0)

 #define DB_PRINT(...) DB_PRINT_L(0, ## __VA_ARGS__)

diff --git a/hw/timer/cadence_ttc.c b/hw/timer/cadence_ttc.c
index 5e65fdb5a0..10056407ab 100644
--- a/hw/timer/cadence_ttc.c
+++ b/hw/timer/cadence_ttc.c
@@ -24,7 +24,7 @@
 #define DB_PRINT(...) do { \
     fprintf(stderr,  ": %s: ", __func__); \
     fprintf(stderr, ## __VA_ARGS__); \
-    } while (0);
+    } while (0)
 #else
     #define DB_PRINT(...)
 #endif
diff --git a/hw/timer/mss-timer.c b/hw/timer/mss-timer.c
index 60f1213a3b..4f814572e2 100644
--- a/hw/timer/mss-timer.c
+++ b/hw/timer/mss-timer.c
@@ -36,7 +36,7 @@
     if (MSS_TIMER_ERR_DEBUG >= lvl) { \
         qemu_log("%s: " fmt "\n", __func__, ## args); \
     } \
-} while (0);
+} while (0)

 #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)

diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
index e5f5e14a90..58fc7b1188 100644
--- a/hw/timer/stm32f2xx_timer.c
+++ b/hw/timer/stm32f2xx_timer.c
@@ -34,7 +34,7 @@
     if (STM_TIMER_ERR_DEBUG >= lvl) { \
         qemu_log("%s: " fmt, __func__, ## args); \
     } \
-} while (0);
+} while (0)

 #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)

diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index c440aff4b2..b8253b338d 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -39,7 +39,7 @@
     if (DEBUG_TPM) { \
         fprintf(stderr, fmt, ## __VA_ARGS__); \
     } \
-} while (0);
+} while (0)

 #define TYPE_TPM_PASSTHROUGH "tpm-passthrough"
 #define TPM_PASSTHROUGH(obj) \
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 42d647d363..6e08cc2033 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -105,7 +105,7 @@ struct TPMState {
     if (DEBUG_TIS) { \
         printf(fmt, ## __VA_ARGS__); \
     } \
-} while (0);
+} while (0)

 /* tis registers */
 #define TPM_TIS_REG_ACCESS                0x00
diff --git a/migration/rdma.c b/migration/rdma.c
index ca56594328..9d5a424011 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -88,7 +88,7 @@ static uint32_t known_capabilities = RDMA_CAPABILITY_PIN_ALL;
             } \
             return rdma->error_state; \
         } \
-    } while (0);
+    } while (0)

 /*
  * A work request ID is 64-bits and we split up these bits
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 625ef2dfd2..89ffd33e87 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -400,7 +400,7 @@ static void unallocated_encoding(DisasContext *s)
                       "at pc=%016" PRIx64 "\n",                          \
                       __FILE__, __LINE__, insn, s->pc - 4);              \
         unallocated_encoding(s);                                         \
-    } while (0);
+    } while (0)

 static void init_tmp_a64_array(DisasContext *s)
 {
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index b03f583032..06d06010a4 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -58,7 +58,7 @@
     if (DEBUG_KVM) {                          \
         fprintf(stderr, fmt, ## __VA_ARGS__); \
     }                                         \
-} while (0);
+} while (0)

 #define kvm_vm_check_mem_attr(s, attr) \
     kvm_vm_check_attr(s, KVM_S390_VM_MEM_CTRL, attr)
diff --git a/tests/tcg/test-mmap.c b/tests/tcg/test-mmap.c
index 3982fa2c72..cdefadfa4c 100644
--- a/tests/tcg/test-mmap.c
+++ b/tests/tcg/test-mmap.c
@@ -39,7 +39,7 @@ do                                                             \
     fprintf (stderr, "FAILED at %s:%d\n", __FILE__, __LINE__); \
     exit (EXIT_FAILURE);                                       \
   }                                                            \
-} while (0);
+} while (0)

 unsigned char *dummybuf;
 static unsigned int pagesize;
-- 
2.14.3

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

[Cornelia Huck]

On Thu, 30 Nov 2017 07:41:59 -0600
Eric Blake <eblake@redhat.com> wrote:

> The point of writing a macro embedded in a 'do { ... } while (0)'
> loop 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;
> the use of do/while (0) in a macro is absolutely essential for the
> purpose of avoiding a syntax error on the 'else' - but it only works
> if there is no trailing ';' in the macro (as the ';' in the code
> calling the macro would then be a second statement and cause the
> 'else' to not pair to the 'if').
> 
> Many of the places touched in this code are examples of the ugly
> bit-rotting debug print statements; cleaning those up is left as
> a bite-sized task for another day.
> 
> Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  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/target/s390x/kvm.c b/target/s390x/kvm.c
> index b03f583032..06d06010a4 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -58,7 +58,7 @@
>      if (DEBUG_KVM) {                          \
>          fprintf(stderr, fmt, ## __VA_ARGS__); \
>      }                                         \
> -} while (0);
> +} while (0)
> 
>  #define kvm_vm_check_mem_attr(s, attr) \
>      kvm_vm_check_attr(s, KVM_S390_VM_MEM_CTRL, attr)

Acked-by: Cornelia Huck <cohuck@redhat.com>

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

[Michael S. Tsirkin]

On Thu, Nov 30, 2017 at 07:41:59AM -0600, Eric Blake wrote:
> The point of writing a macro embedded in a 'do { ... } while (0)'
> loop 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;
> the use of do/while (0) in a macro is absolutely essential for the
> purpose of avoiding a syntax error on the 'else' - but it only works
> if there is no trailing ';' in the macro (as the ';' in the code
> calling the macro would then be a second statement and cause the
> 'else' to not pair to the 'if').

Shouldn't matter if everyone puts the statements in {}, right?

> Many of the places touched in this code are examples of the ugly
> bit-rotting debug print statements; cleaning those up is left as
> a bite-sized task for another day.
> 
> Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

We can't really rely on code still building for this to do the right
thing, can we?  I did my best to look for uses and I think it's OK, so

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

but I'm not merging this.

> -- 
> 2.14.3

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

[Eric Blake]

On 11/30/2017 08:43 AM, Michael S. Tsirkin wrote:
> On Thu, Nov 30, 2017 at 07:41:59AM -0600, Eric Blake wrote:
>> The point of writing a macro embedded in a 'do { ... } while (0)'
>> loop 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;
>> the use of do/while (0) in a macro is absolutely essential for the
>> purpose of avoiding a syntax error on the 'else' - but it only works
>> if there is no trailing ';' in the macro (as the ';' in the code
>> calling the macro would then be a second statement and cause the
>> 'else' to not pair to the 'if').
> 
> Shouldn't matter if everyone puts the statements in {}, right?

Correct - where we follow our style, spurious ';' don't make a 
difference (other than they might trigger a warning in a very particular 
compiler).  But it also makes our code harder to copy-and-paste into 
other projects with a different style.

> 
>> Many of the places touched in this code are examples of the ugly
>> bit-rotting debug print statements; cleaning those up is left as
>> a bite-sized task for another day.
>>
>> Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> We can't really rely on code still building for this to do the right
> thing, can we?

Some of the uses that were changed are in dead #ifdefs for debugging 
purposes - the only way to still compile is to turn on the debugging, 
but that may fail to compile for other reasons (if the format strings 
have bit-rotted, for example).

The only sure way to know that this did not break anything is to audit 
that for every macro where I eliminated the ';', all callers of that 
macro call 'foo();'.  I did not perform that audit (the patch was 
mechanical) - but am willing to do so and reply back with results if you 
need the extra confidence.

>  I did my best to look for uses and I think it's OK, so
> 
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> 
> but I'm not merging this.

What tree should it go through instead? Does it need to be split along 
maintainer boundaries?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> The point of writing a macro embedded in a 'do { ... } while (0)'
> loop 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;
> the use of do/while (0) in a macro is absolutely essential for the
> purpose of avoiding a syntax error on the 'else' - but it only works
> if there is no trailing ';' in the macro (as the ';' in the code
> calling the macro would then be a second statement and cause the
> 'else' to not pair to the 'if').
> 
> Many of the places touched in this code are examples of the ugly
> bit-rotting debug print statements; cleaning those up is left as
> a bite-sized task for another day.
> 
> Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---

> diff --git a/migration/rdma.c b/migration/rdma.c
> index ca56594328..9d5a424011 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -88,7 +88,7 @@ static uint32_t known_capabilities = RDMA_CAPABILITY_PIN_ALL;
>              } \
>              return rdma->error_state; \
>          } \
> -    } while (0);
> +    } while (0)

Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]

On Thu, Nov 30, 2017 at 07:41:59AM -0600, Eric Blake wrote:
> The point of writing a macro embedded in a 'do { ... } while (0)'
> loop 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;
> the use of do/while (0) in a macro is absolutely essential for the
> purpose of avoiding a syntax error on the 'else' - but it only works
> if there is no trailing ';' in the macro (as the ';' in the code
> calling the macro would then be a second statement and cause the
> 'else' to not pair to the 'if').
> 
> Many of the places touched in this code are examples of the ugly
> bit-rotting debug print statements; cleaning those up is left as
> a bite-sized task for another day.
> 
> Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

ppc related portions

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[Qemu-devel] [PATCH 4/3] checkpatch: Enforce proper do/while (0) style

[Eric Blake]

while (0) is only idiomatic in a macro definition, where the caller
will be supplying the trailing ';'.  Warn if the macro has a duplicate.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/checkpatch.pl | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 34df753571..acb66bff34 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1622,6 +1622,11 @@ sub process {
 			}
 		}

+# 'while (0);' is odd; only macros should use while (0), without trailing ;
+		if ($line =~ /while\s*\(0\);/) {
+			ERROR("suspicious ; after while (0)\n" . $herecurr);
+		}
+
 # Check relative indent for conditionals and blocks.
 		if ($line =~ /\b(?:(?:if|while|for)\s*\(|do\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) {
 			my ($s, $c) = ($stat, $cond);
-- 
2.14.3

Re: [Qemu-devel] [PATCH 4/3] checkpatch: Enforce proper do/while (0) style

[Eric Blake]

On 11/30/2017 08:01 AM, Eric Blake wrote:
> while (0) is only idiomatic in a macro definition, where the caller
> will be supplying the trailing ';'.  Warn if the macro has a duplicate.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   scripts/checkpatch.pl | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 34df753571..acb66bff34 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1622,6 +1622,11 @@ sub process {
>   			}
>   		}
> 
> +# 'while (0);' is odd; only macros should use while (0), without trailing ;
> +		if ($line =~ /while\s*\(0\);/) {

Should this also check for uses of 'while (false);' ?

Interestingly enough, we have an instance of 'do/while (false);' in 
tests/vhost-user-bridge.c that is NOT in a macro, but is used for the 
convenience of being able to 'break;' out early rather than using a 
goto.  Similarly for chardev/char-serial.c using 'while (0);' outside of 
a macro.  Those may be worth rewriting to use goto as separate patches 
if we want to restrict ALL use of 'while \((0|false)\);'

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 4/3] checkpatch: Enforce proper do/while (0) style

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 11/30/2017 08:01 AM, Eric Blake wrote:
>> while (0) is only idiomatic in a macro definition, where the caller
>> will be supplying the trailing ';'.  Warn if the macro has a duplicate.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>   scripts/checkpatch.pl | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 34df753571..acb66bff34 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -1622,6 +1622,11 @@ sub process {
>>   			}
>>   		}
>>
>> +# 'while (0);' is odd; only macros should use while (0), without trailing ;
>> +		if ($line =~ /while\s*\(0\);/) {
>
> Should this also check for uses of 'while (false);' ?

Do we think it's likely to occur?

> Interestingly enough, we have an instance of 'do/while (false);' in
> tests/vhost-user-bridge.c that is NOT in a macro, but is used for the
> convenience of being able to 'break;' out early rather than using a
> goto.  Similarly for chardev/char-serial.c using 'while (0);' outside
> of a macro.

That "cure" merely adds gratuitous cleverness to the "disease".

>              Those may be worth rewriting to use goto as separate
> patches if we want to restrict ALL use of 'while \((0|false)\);'

I'd support that.

Re: [Qemu-devel] [PATCH 4/3] checkpatch: Enforce proper do/while (0) style

[Eric Blake]

On 12/01/2017 01:31 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 11/30/2017 08:01 AM, Eric Blake wrote:
>>> while (0) is only idiomatic in a macro definition, where the caller
>>> will be supplying the trailing ';'.  Warn if the macro has a duplicate.
>>>

>>>
>>> +# 'while (0);' is odd; only macros should use while (0), without trailing ;
>>> +		if ($line =~ /while\s*\(0\);/) {
>>
>> Should this also check for uses of 'while (false);' ?
> 
> Do we think it's likely to occur?

Not as frequent, but it does appear in the code base.

> 
>> Interestingly enough, we have an instance of 'do/while (false);' in
>> tests/vhost-user-bridge.c that is NOT in a macro, but is used for the
>> convenience of being able to 'break;' out early rather than using a
>> goto.  Similarly for chardev/char-serial.c using 'while (0);' outside
>> of a macro.
> 
> That "cure" merely adds gratuitous cleverness to the "disease".
> 
>>               Those may be worth rewriting to use goto as separate
>> patches if we want to restrict ALL use of 'while \((0|false)\);'
> 
> I'd support that.
> 

Okay, I'll post a v2 along those lines.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org