macro do/while (0) cleanup

PATCH Applied Eric Blake 4 participants 13 messages Dec 1, 2017 – Jan 10, 2018

Subsystems (3)

Arm STM32 Arm/Zynq architecture Checkpatch

lore

Share & tools

lore

Eric Blake

author

2017-12-01 23:24 UTC #0 lore raw

v2 cover macro do/while (0) cleanup

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(-)

Signature · 1 line

2.14.3

Eric Blake

author

2017-12-01 23:24 UTC #1 lore raw

PATCH v2 1/7 net: Drop unusual use of do { } while (0);

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 <[email protected]>

Reviewed-by Thomas Huth <[email protected]>

---
 hw/net/pcnet.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

hw/net/pcnet.c +10 ~~-10~~

index 654455355f..641bf2fd88 100644
@@ -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)

Signature · 1 line

2.14.3

Eric Blake

author

2017-12-01 23:24 UTC #2 lore raw

PATCH v2 2/7 mips: Tweak location of ';' in macros

In reply to 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 <[email protected]>

Reviewed-by Philippe Mathieu-Daudé <[email protected]>

---
 target/mips/msa_helper.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

target/mips/msa_helper.c +18 ~~-16~~

index f167a42655..8fb7a369ca 100644
@@ -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

Signature · 1 line

2.14.3

Eric Blake

author

2017-12-01 23:24 UTC #3 lore raw

PATCH v2 3/7 chardev: Use goto/label instead of do/break/while(0)

In reply to 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 <[email protected]>

---
 chardev/char-serial.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

chardev/char-serial.c +10 -3

index 2f8f83821d..10162f9fa3 100644
@@ -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);

Signature · 1 line

2.14.3

Marc-André Lureau

Reviewed-by on v2 3/7

2017-12-03 14:20 UTC #4 lore raw

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

Quoted 8 lines · 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 <[email protected]>
---

Reviewed-by Marc-André Lureau <[email protected]>

Quoted 40 lines

 chardev/char-serial.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

chardev/char-serial.c +10 -4

index 2f8f83821d..10162f9fa3 100644
@@ -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

Signature · 1 line

Marc-André Lureau

Eric Blake

author

2017-12-01 23:24 UTC #5 lore raw

PATCH v2 4/7 chardev: Clean up previous patch indentation

In reply to 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 <[email protected]>

---
 chardev/char-serial.c | 66 +++++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 34 deletions(-)

chardev/char-serial.c +32 ~~-34~~

index 10162f9fa3..93392c528c 100644
@@ -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:

Signature · 1 line

2.14.3

Marc-André Lureau

Reviewed-by on v2 4/7

2017-12-03 14:20 UTC #6 lore raw

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

Quoted 5 lines · 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 <[email protected]>

Reviewed-by Marc-André Lureau <[email protected]>

---

Quoted 110 lines

 chardev/char-serial.c | 66 +++++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 34 deletions(-)

chardev/char-serial.c +32 ~~-35~~

index 10162f9fa3..93392c528c 100644
@@ -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

Signature · 1 line

Marc-André Lureau

Eric Blake

author

2017-12-01 23:24 UTC #7 lore raw

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

In reply to 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 <[email protected]>

---
 tests/vhost-user-bridge.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

tests/vhost-user-bridge.c +4 -2

index d820033a72..e0605a529e 100644
@@ -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;

Signature · 1 line

2.14.3

Eric Blake

author

2017-12-01 23:25 UTC #8 lore raw

PATCH v2 6/7 maint: Fix macros with broken 'do/while(0); ' usage

In reply to 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 <[email protected]>

Acked-by Cornelia Huck <[email protected]>

Reviewed-by Michael S. Tsirkin <[email protected]>

Acked-by Dr. David Alan Gilbert <[email protected]>

---
 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(-)

tests/acpi-utils.h +4 -4

index d5ca5b6238..ac52abd0dd 100644
@@ -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);

ui/sdl_zoom_template.h +4 -4

index 3bb508b51e..6a424adfb4 100644
@@ -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)

audio/paaudio.c +2 -2

index 65beb6f010..2a35e6f82c 100644
@@ -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)
 {

hw/adc/stm32f2xx_adc.c +1 -1

index 90fe9de299..13f31ad2f7 100644
@@ -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)

hw/block/m25p80.c +1 -1

index a2438b9ed2..62b5221bde 100644
@@ -40,7 +40,7 @@
         fprintf(stderr,  ": %s: ", __func__); \
         fprintf(stderr, ## __VA_ARGS__); \
     } \
-} while (0);
+} while (0)
 
 /* Fields for FlashPartInfo->flags */

hw/char/cadence_uart.c +1 -1

index 6143494060..fbdbd463bb 100644
@@ -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

hw/char/stm32f2xx_usart.c +1 -1

index 268e435338..07b462d4b6 100644
@@ -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)

hw/display/cg3.c +1 -1

index e069c4484c..cafd9f47ef 100644
@@ -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)

hw/display/dpcd.c +1 -1

index ce92ff6e2a..943002bee5 100644
@@ -39,7 +39,7 @@
     if (DEBUG_DPCD) {                                                          \
         qemu_log("dpcd: " fmt, ## __VA_ARGS__);                                \
     }                                                                          \
-} while (0);
+} while (0)
 
 #define DPCD_READABLE_AREA                      0x600

hw/display/xlnx_dp.c +1 -1

index 561f828e7a..ead4e1a0e4 100644
@@ -34,7 +34,7 @@
     if (DEBUG_DP) {                                                            \
         qemu_log("xlnx_dp: " fmt , ## __VA_ARGS__);                            \
     }                                                                          \
-} while (0);
+} while (0)
 
 /*
  * Register offset for DP.

hw/dma/pl330.c +1 -1

index 32cf8399b8..d071049233 100644
@@ -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)

hw/dma/xlnx-zynq-devcfg.c +1 -1

index 3b10523430..12bb2e3716 100644
@@ -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 */

hw/dma/xlnx_dpdma.c +1 -1

index 8ceb21ddb3..077c7da9cc 100644
@@ -34,7 +34,7 @@
     if (DEBUG_DPDMA) {                                                         \
         qemu_log("xlnx_dpdma: " fmt , ## __VA_ARGS__);                         \
     }                                                                          \
-} while (0);
+} while (0)
 
 /*
  * Registers offset for DPDMA.

hw/i2c/i2c-ddc.c +1 -1

index 6b92e95c73..199dac9e41 100644
@@ -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()

hw/misc/auxbus.c +1 -1

index 1182745044..b4cacd664b 100644
@@ -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)

hw/misc/macio/mac_dbdma.c +2 -2

index 0eddf2e700..1b2a69b3ef 100644
@@ -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)
 
 /*
  */

hw/misc/mmio_interface.c +1 -1

index 894e9801cb..3b0e2039a3 100644
@@ -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)
 {

hw/misc/stm32f2xx_syscfg.c +1 -1

index 7c45833d09..7f10195862 100644
@@ -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)

hw/misc/zynq_slcr.c +1 -1

index 44304d48be..d6bdd027ef 100644
@@ -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

hw/net/cadence_gem.c +1 -1

index 3943187572..0fa4b0dc44 100644
@@ -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

hw/ssi/mss-spi.c +1 -1

index d60daba882..185e1a3920 100644
@@ -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)

hw/ssi/stm32f2xx_spi.c +1 -1

index 26a1b4ddf5..69514da9fb 100644
@@ -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)

hw/ssi/xilinx_spi.c +1 -1

index 33482f04de..83585bc8b2 100644
@@ -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

hw/ssi/xilinx_spips.c +1 -1

index ef56d35f2c..c070393dc9 100644
@@ -43,7 +43,7 @@
         fprintf(stderr,  ": %s: ", __func__); \
         fprintf(stderr, ## __VA_ARGS__); \
     } \
-} while (0);
+} while (0)
 
 /* config register */
 #define R_CONFIG            (0x00 / 4)

hw/timer/a9gtimer.c +1 -1

index ce1dc63911..96d534d8a8 100644
@@ -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__)

hw/timer/cadence_ttc.c +1 -1

index 5e65fdb5a0..10056407ab 100644
@@ -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

hw/timer/mss-timer.c +1 -1

index 60f1213a3b..4f814572e2 100644
@@ -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)

hw/timer/stm32f2xx_timer.c +1 -1

index e5f5e14a90..58fc7b1188 100644
@@ -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)

hw/tpm/tpm_passthrough.c +1 -1

index c440aff4b2..b8253b338d 100644
@@ -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) \

hw/tpm/tpm_tis.c +1 -1

index 42d647d363..6e08cc2033 100644
@@ -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

migration/rdma.c +1 -1

index ca56594328..9d5a424011 100644
@@ -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

target/arm/translate-a64.c +1 -1

index 625ef2dfd2..89ffd33e87 100644
@@ -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)
 {

target/s390x/kvm.c +1 -1

index b03f583032..06d06010a4 100644
@@ -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)

tests/tcg/test-mmap.c +1 -1

index 3982fa2c72..cdefadfa4c 100644
@@ -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;

Signature · 1 line

2.14.3

Juan Quintela

Reviewed-by on v2 6/7

2017-12-02 11:53 UTC #9 lore raw

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

Quoted 29 lines · 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.

Quoted 6 lines

Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
Signed-off-by Eric Blake <[email protected]>
Acked-by Cornelia Huck <[email protected]>
Reviewed-by Michael S. Tsirkin <[email protected]>
Acked-by Dr. David Alan Gilbert <[email protected]>

Reviewed-by Juan Quintela <[email protected]>

Eric Blake

author

2017-12-01 23:25 UTC #10 lore raw

PATCH v2 7/7 checkpatch: Enforce proper do/while (0) style

In reply to 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 <[email protected]>

---
 scripts/checkpatch.pl | 5 +++++
 1 file changed, 5 insertions(+)

scripts/checkpatch.pl +5 -0

index 34df753571..27800a4ed9 100755
@@ -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);

Signature · 1 line

2.14.3

Eric Blake

author

2018-01-09 20:52 UTC #11 lore raw

In reply to Eric Blake

ping

Quoted 67 lines · 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(-)

Signature · 3 lines

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

Paolo Bonzini

Applied

2018-01-10 13:52 UTC #12 lore raw

Quoted 1 line · Eric Blake

ping

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

Queued now, thanks.

Paolo

Quoted 69 lines · Eric Blake

Quoted 67 lines
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(-)