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

[Qemu-devel] [PATCH v2 0/7] 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^wKNOW 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.

In v2:
- add review tags
- audit that the maint patch has no semantic change (all callers
were indeed supplying ';' at the macro call-sites)
- improved commit messages
- more patches for some additional cleanups
- tighten the checkpatch rule to also flag 'while (false);'

Eric Blake (7):
  net: Drop unusual use of do { } while (0);
  mips: Tweak location of ';' in macros
  chardev: Use goto/label instead of do/break/while(0)
  chardev: Clean up previous patch indentation
  tests: Avoid 'do/while(false);' in vhost-user-bridge
  maint: Fix macros with broken 'do/while(0);' usage
  checkpatch: Enforce proper do/while (0) style

 tests/acpi-utils.h         |  8 ++---
 ui/sdl_zoom_template.h     |  8 ++---
 audio/paaudio.c            |  4 +--
 chardev/char-serial.c      | 75 ++++++++++++++++++++++++----------------------
 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 +-
 tests/vhost-user-bridge.c  |  6 ++--
 scripts/checkpatch.pl      |  5 ++++
 39 files changed, 119 insertions(+), 105 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v2 1/7] 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>
Reviewed-by: Thomas Huth <thuth@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

[Qemu-devel] [PATCH v2 2/7] 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>
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
-- 
2.14.3

[Qemu-devel] [PATCH v2 3/7] chardev: Use goto/label instead of do/break/while(0)

[Eric Blake]

Use of a do/while(0) control flow in order to permit an early break
is an unusual paradigm, and triggers a false positive with a planned
future syntax check against 'while (0);'.  Rewrite the code to use a
goto instead.  This patch temporarily keeps an extra level of
indentation to highlight the change; the next patch cleans it up.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 chardev/char-serial.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/chardev/char-serial.c b/chardev/char-serial.c
index 2f8f83821d..10162f9fa3 100644
--- a/chardev/char-serial.c
+++ b/chardev/char-serial.c
@@ -64,9 +64,14 @@ static void tty_serial_init(int fd, int speed,
 #endif
     tcgetattr(fd, &tty);

-#define check_speed(val) if (speed <= val) { spd = B##val; break; }
+#define check_speed(val) \
+    if (speed <= val) {  \
+        spd = B##val;    \
+        goto done;       \
+    }
+
     speed = speed * 10 / 11;
-    do {
+    {
         check_speed(50);
         check_speed(75);
         check_speed(110);
@@ -125,8 +130,10 @@ static void tty_serial_init(int fd, int speed,
         check_speed(4000000);
 #endif
         spd = B115200;
-    } while (0);
+    }

+#undef check_speed
+ done:
     cfsetispeed(&tty, spd);
     cfsetospeed(&tty, spd);

-- 
2.14.3

Re: [Qemu-devel] [PATCH v2 3/7] chardev: Use goto/label instead of do/break/while(0)

[Marc-André Lureau]

On Sat, Dec 2, 2017 at 12:24 AM, Eric Blake <eblake@redhat.com> wrote:
> Use of a do/while(0) control flow in order to permit an early break
> is an unusual paradigm, and triggers a false positive with a planned
> future syntax check against 'while (0);'.  Rewrite the code to use a
> goto instead.  This patch temporarily keeps an extra level of
> indentation to highlight the change; the next patch cleans it up.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>  chardev/char-serial.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/chardev/char-serial.c b/chardev/char-serial.c
> index 2f8f83821d..10162f9fa3 100644
> --- a/chardev/char-serial.c
> +++ b/chardev/char-serial.c
> @@ -64,9 +64,14 @@ static void tty_serial_init(int fd, int speed,
>  #endif
>      tcgetattr(fd, &tty);
>
> -#define check_speed(val) if (speed <= val) { spd = B##val; break; }
> +#define check_speed(val) \
> +    if (speed <= val) {  \
> +        spd = B##val;    \
> +        goto done;       \
> +    }
> +
>      speed = speed * 10 / 11;
> -    do {
> +    {
>          check_speed(50);
>          check_speed(75);
>          check_speed(110);
> @@ -125,8 +130,10 @@ static void tty_serial_init(int fd, int speed,
>          check_speed(4000000);
>  #endif
>          spd = B115200;
> -    } while (0);
> +    }
>
> +#undef check_speed
> + done:
>      cfsetispeed(&tty, spd);
>      cfsetospeed(&tty, spd);
>
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

[Qemu-devel] [PATCH v2 4/7] chardev: Clean up previous patch indentation

[Eric Blake]

The previous patch left in an extra scope layer for ease of
review; time to remove it.  No semantic change.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 chardev/char-serial.c | 66 +++++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/chardev/char-serial.c b/chardev/char-serial.c
index 10162f9fa3..93392c528c 100644
--- a/chardev/char-serial.c
+++ b/chardev/char-serial.c
@@ -71,66 +71,64 @@ static void tty_serial_init(int fd, int speed,
     }

     speed = speed * 10 / 11;
-    {
-        check_speed(50);
-        check_speed(75);
-        check_speed(110);
-        check_speed(134);
-        check_speed(150);
-        check_speed(200);
-        check_speed(300);
-        check_speed(600);
-        check_speed(1200);
-        check_speed(1800);
-        check_speed(2400);
-        check_speed(4800);
-        check_speed(9600);
-        check_speed(19200);
-        check_speed(38400);
-        /* Non-Posix values follow. They may be unsupported on some systems. */
-        check_speed(57600);
-        check_speed(115200);
+    check_speed(50);
+    check_speed(75);
+    check_speed(110);
+    check_speed(134);
+    check_speed(150);
+    check_speed(200);
+    check_speed(300);
+    check_speed(600);
+    check_speed(1200);
+    check_speed(1800);
+    check_speed(2400);
+    check_speed(4800);
+    check_speed(9600);
+    check_speed(19200);
+    check_speed(38400);
+    /* Non-Posix values follow. They may be unsupported on some systems. */
+    check_speed(57600);
+    check_speed(115200);
 #ifdef B230400
-        check_speed(230400);
+    check_speed(230400);
 #endif
 #ifdef B460800
-        check_speed(460800);
+    check_speed(460800);
 #endif
 #ifdef B500000
-        check_speed(500000);
+    check_speed(500000);
 #endif
 #ifdef B576000
-        check_speed(576000);
+    check_speed(576000);
 #endif
 #ifdef B921600
-        check_speed(921600);
+    check_speed(921600);
 #endif
 #ifdef B1000000
-        check_speed(1000000);
+    check_speed(1000000);
 #endif
 #ifdef B1152000
-        check_speed(1152000);
+    check_speed(1152000);
 #endif
 #ifdef B1500000
-        check_speed(1500000);
+    check_speed(1500000);
 #endif
 #ifdef B2000000
-        check_speed(2000000);
+    check_speed(2000000);
 #endif
 #ifdef B2500000
-        check_speed(2500000);
+    check_speed(2500000);
 #endif
 #ifdef B3000000
-        check_speed(3000000);
+    check_speed(3000000);
 #endif
 #ifdef B3500000
-        check_speed(3500000);
+    check_speed(3500000);
 #endif
 #ifdef B4000000
-        check_speed(4000000);
+    check_speed(4000000);
 #endif
-        spd = B115200;
-    }
+    spd = B115200;

 #undef check_speed
  done:
-- 
2.14.3

Re: [Qemu-devel] [PATCH v2 4/7] chardev: Clean up previous patch indentation

[Marc-André Lureau]

On Sat, Dec 2, 2017 at 12:24 AM, Eric Blake <eblake@redhat.com> wrote:
> The previous patch left in an extra scope layer for ease of
> review; time to remove it.  No semantic change.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


 ---
>  chardev/char-serial.c | 66 +++++++++++++++++++++++++--------------------------
>  1 file changed, 32 insertions(+), 34 deletions(-)
>
> diff --git a/chardev/char-serial.c b/chardev/char-serial.c
> index 10162f9fa3..93392c528c 100644
> --- a/chardev/char-serial.c
> +++ b/chardev/char-serial.c
> @@ -71,66 +71,64 @@ static void tty_serial_init(int fd, int speed,
>      }
>
>      speed = speed * 10 / 11;
> -    {
> -        check_speed(50);
> -        check_speed(75);
> -        check_speed(110);
> -        check_speed(134);
> -        check_speed(150);
> -        check_speed(200);
> -        check_speed(300);
> -        check_speed(600);
> -        check_speed(1200);
> -        check_speed(1800);
> -        check_speed(2400);
> -        check_speed(4800);
> -        check_speed(9600);
> -        check_speed(19200);
> -        check_speed(38400);
> -        /* Non-Posix values follow. They may be unsupported on some systems. */
> -        check_speed(57600);
> -        check_speed(115200);
> +    check_speed(50);
> +    check_speed(75);
> +    check_speed(110);
> +    check_speed(134);
> +    check_speed(150);
> +    check_speed(200);
> +    check_speed(300);
> +    check_speed(600);
> +    check_speed(1200);
> +    check_speed(1800);
> +    check_speed(2400);
> +    check_speed(4800);
> +    check_speed(9600);
> +    check_speed(19200);
> +    check_speed(38400);
> +    /* Non-Posix values follow. They may be unsupported on some systems. */
> +    check_speed(57600);
> +    check_speed(115200);
>  #ifdef B230400
> -        check_speed(230400);
> +    check_speed(230400);
>  #endif
>  #ifdef B460800
> -        check_speed(460800);
> +    check_speed(460800);
>  #endif
>  #ifdef B500000
> -        check_speed(500000);
> +    check_speed(500000);
>  #endif
>  #ifdef B576000
> -        check_speed(576000);
> +    check_speed(576000);
>  #endif
>  #ifdef B921600
> -        check_speed(921600);
> +    check_speed(921600);
>  #endif
>  #ifdef B1000000
> -        check_speed(1000000);
> +    check_speed(1000000);
>  #endif
>  #ifdef B1152000
> -        check_speed(1152000);
> +    check_speed(1152000);
>  #endif
>  #ifdef B1500000
> -        check_speed(1500000);
> +    check_speed(1500000);
>  #endif
>  #ifdef B2000000
> -        check_speed(2000000);
> +    check_speed(2000000);
>  #endif
>  #ifdef B2500000
> -        check_speed(2500000);
> +    check_speed(2500000);
>  #endif
>  #ifdef B3000000
> -        check_speed(3000000);
> +    check_speed(3000000);
>  #endif
>  #ifdef B3500000
> -        check_speed(3500000);
> +    check_speed(3500000);
>  #endif
>  #ifdef B4000000
> -        check_speed(4000000);
> +    check_speed(4000000);
>  #endif
> -        spd = B115200;
> -    }
> +    spd = B115200;
>
>  #undef check_speed
>   done:
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

[Qemu-devel] [PATCH v2 5/7] tests: Avoid 'do/while(false); ' in vhost-user-bridge

[Eric Blake]

Use of a do/while(0) loop as a way to allow break statements in
the middle of execute-once code is unusual.  More typical is
the use of goto for early exits, with a label at the end of
the execute-once code, rather than nesting code in a scope;
however, the comment at the end of the existing code makes this
alternative a bit unpractical.

So, to avoid false positives from a future syntax check about
'while (false);', and to keep the loop form (in case someone
ever does add DONTWAIT support, where they can just as easily
manipulate the initial loop condition or add an if around the
final 'break'), I opted to use the form of a while(1) loop (the
break as an early exit is more idiomatic there), coupled with
a final break preserving the original comment.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/vhost-user-bridge.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
index d820033a72..e0605a529e 100644
--- a/tests/vhost-user-bridge.c
+++ b/tests/vhost-user-bridge.c
@@ -283,7 +283,7 @@ vubr_backend_recv_cb(int sock, void *ctx)
         return;
     }

-    do {
+    while (1) {
         struct iovec *sg;
         ssize_t ret, total = 0;
         unsigned int num;
@@ -343,7 +343,9 @@ vubr_backend_recv_cb(int sock, void *ctx)

         free(elem);
         elem = NULL;
-    } while (false); /* could loop if DONTWAIT worked? */
+
+        break;        /* could loop if DONTWAIT worked? */
+    }

     if (mhdr_cnt) {
         mhdr.num_buffers = i;
-- 
2.14.3

[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 <eblake@redhat.com>
Acked-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Dr. David Alan Gilbert <dgilbert@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 v2 6/7] maint: Fix macros with broken 'do/while(0); ' usage

[Juan Quintela]

Eric Blake <eblake@redhat.com> 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 <eblake@redhat.com>
> Acked-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

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

[Eric Blake]

Use of a loop construct for code that is not intended to repeat
does not make much idiomatic sense, except in one place: it is a
common usage in macros in order to wrap arbitrary code with
single-statement semantics.  But when used in a macro, it is more
typical for the caller to supply the trailing ';' when calling
the macro.

Although qemu coding style frowns on bare:
  if (cond)
    statement1;
  else
    statement2;
where extra semicolons actually cause syntax errors, we still
want our macro styles to be easily copied to other projects.
Thus, declare it an error if we encounter any form of 'while (0)'
with a semicolon in the same line.

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..27800a4ed9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1622,6 +1622,11 @@ sub process {
 			}
 		}

+# 'do ... while (0/false)' only makes sense in macros, without trailing ';'
+		if ($line =~ /while\s*\((0|false)\);/) {
+			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 v2 0/7] macro do/while (0) cleanup

[Eric Blake]

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

ping

On 12/01/2017 05:24 PM, Eric Blake wrote:
> Noticed this by chance in the tests/ directory, so I broadened
> it to a grep of the entire code base.  I suspect^wKNOW 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.
> 
> In v2:
> - add review tags
> - audit that the maint patch has no semantic change (all callers
> were indeed supplying ';' at the macro call-sites)
> - improved commit messages
> - more patches for some additional cleanups
> - tighten the checkpatch rule to also flag 'while (false);'
> 
> Eric Blake (7):
>   net: Drop unusual use of do { } while (0);
>   mips: Tweak location of ';' in macros
>   chardev: Use goto/label instead of do/break/while(0)
>   chardev: Clean up previous patch indentation
>   tests: Avoid 'do/while(false);' in vhost-user-bridge
>   maint: Fix macros with broken 'do/while(0);' usage
>   checkpatch: Enforce proper do/while (0) style
> 
>  tests/acpi-utils.h         |  8 ++---
>  ui/sdl_zoom_template.h     |  8 ++---
>  audio/paaudio.c            |  4 +--
>  chardev/char-serial.c      | 75 ++++++++++++++++++++++++----------------------
>  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 +-
>  tests/vhost-user-bridge.c  |  6 ++--
>  scripts/checkpatch.pl      |  5 ++++
>  39 files changed, 119 insertions(+), 105 deletions(-)
> 

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup

[Paolo Bonzini]

On 09/01/2018 21:52, Eric Blake wrote:
> ping

Hmm, the actual posting of the patches wasn't CCed to qemu-trivial.

Queued now, thanks.

Paolo

> On 12/01/2017 05:24 PM, Eric Blake wrote:
>> Noticed this by chance in the tests/ directory, so I broadened
>> it to a grep of the entire code base.  I suspect^wKNOW 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.
>>
>> In v2:
>> - add review tags
>> - audit that the maint patch has no semantic change (all callers
>> were indeed supplying ';' at the macro call-sites)
>> - improved commit messages
>> - more patches for some additional cleanups
>> - tighten the checkpatch rule to also flag 'while (false);'
>>
>> Eric Blake (7):
>>   net: Drop unusual use of do { } while (0);
>>   mips: Tweak location of ';' in macros
>>   chardev: Use goto/label instead of do/break/while(0)
>>   chardev: Clean up previous patch indentation
>>   tests: Avoid 'do/while(false);' in vhost-user-bridge
>>   maint: Fix macros with broken 'do/while(0);' usage
>>   checkpatch: Enforce proper do/while (0) style
>>
>>  tests/acpi-utils.h         |  8 ++---
>>  ui/sdl_zoom_template.h     |  8 ++---
>>  audio/paaudio.c            |  4 +--
>>  chardev/char-serial.c      | 75 ++++++++++++++++++++++++----------------------
>>  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 +-
>>  tests/vhost-user-bridge.c  |  6 ++--
>>  scripts/checkpatch.pl      |  5 ++++
>>  39 files changed, 119 insertions(+), 105 deletions(-)
>>
>