* [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup
@ 2017-12-01 23:24 Eric Blake
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 1/7] net: Drop unusual use of do { } while (0); Eric Blake
` (7 more replies)
0 siblings, 8 replies; 13+ messages in thread
From: Eric Blake @ 2017-12-01 23:24 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, qemu-trivial, pbonzini
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 1/7] net: Drop unusual use of do { } while (0);
2017-12-01 23:24 [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
@ 2017-12-01 23:24 ` Eric Blake
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 2/7] mips: Tweak location of ';' in macros Eric Blake
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Eric Blake @ 2017-12-01 23:24 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, qemu-trivial, pbonzini, Jason Wang
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 2/7] mips: Tweak location of ';' in macros
2017-12-01 23:24 [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 1/7] net: Drop unusual use of do { } while (0); Eric Blake
@ 2017-12-01 23:24 ` Eric Blake
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 3/7] chardev: Use goto/label instead of do/break/while(0) Eric Blake
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Eric Blake @ 2017-12-01 23:24 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, qemu-trivial, pbonzini, Aurelien Jarno, Yongbok Kim
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 3/7] chardev: Use goto/label instead of do/break/while(0)
2017-12-01 23:24 [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 1/7] net: Drop unusual use of do { } while (0); Eric Blake
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 2/7] mips: Tweak location of ';' in macros Eric Blake
@ 2017-12-01 23:24 ` Eric Blake
2017-12-03 14:20 ` Marc-André Lureau
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 4/7] chardev: Clean up previous patch indentation Eric Blake
` (4 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Eric Blake @ 2017-12-01 23:24 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, qemu-trivial, pbonzini, Marc-André Lureau
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 4/7] chardev: Clean up previous patch indentation
2017-12-01 23:24 [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
` (2 preceding siblings ...)
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 3/7] chardev: Use goto/label instead of do/break/while(0) Eric Blake
@ 2017-12-01 23:24 ` Eric Blake
2017-12-03 14:20 ` Marc-André Lureau
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 5/7] tests: Avoid 'do/while(false); ' in vhost-user-bridge Eric Blake
` (3 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Eric Blake @ 2017-12-01 23:24 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, qemu-trivial, pbonzini, Marc-André Lureau
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 5/7] tests: Avoid 'do/while(false); ' in vhost-user-bridge
2017-12-01 23:24 [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
` (3 preceding siblings ...)
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 4/7] chardev: Clean up previous patch indentation Eric Blake
@ 2017-12-01 23:24 ` Eric Blake
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 6/7] maint: Fix macros with broken 'do/while(0); ' usage Eric Blake
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Eric Blake @ 2017-12-01 23:24 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, qemu-trivial, pbonzini
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 6/7] maint: Fix macros with broken 'do/while(0); ' usage
2017-12-01 23:24 [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
` (4 preceding siblings ...)
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 5/7] tests: Avoid 'do/while(false); ' in vhost-user-bridge Eric Blake
@ 2017-12-01 23:24 ` Eric Blake
2017-12-02 11:53 ` Juan Quintela
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 7/7] checkpatch: Enforce proper do/while (0) style Eric Blake
2018-01-09 20:52 ` [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
7 siblings, 1 reply; 13+ messages in thread
From: Eric Blake @ 2017-12-01 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: armbru, qemu-trivial, pbonzini, Gerd Hoffmann, Alistair Francis,
Peter Crosthwaite, Kevin Wolf, Max Reitz, Edgar E. Iglesias,
Peter Maydell, Alexander Graf, Jason Wang, Subbaraya Sundeep,
Stefan Berger, Juan Quintela, Dr. David Alan Gilbert,
Christian Borntraeger, Cornelia Huck, Richard Henderson,
Michael S. Tsirkin, Igor Mammedov, open list:Block layer core,
open list:Xilinx Zynq, open list:New World, open list:S390
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 7/7] checkpatch: Enforce proper do/while (0) style
2017-12-01 23:24 [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
` (5 preceding siblings ...)
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 6/7] maint: Fix macros with broken 'do/while(0); ' usage Eric Blake
@ 2017-12-01 23:24 ` Eric Blake
2018-01-09 20:52 ` [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
7 siblings, 0 replies; 13+ messages in thread
From: Eric Blake @ 2017-12-01 23:24 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, qemu-trivial, pbonzini
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] maint: Fix macros with broken 'do/while(0); ' usage
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 6/7] maint: Fix macros with broken 'do/while(0); ' usage Eric Blake
@ 2017-12-02 11:53 ` Juan Quintela
0 siblings, 0 replies; 13+ messages in thread
From: Juan Quintela @ 2017-12-02 11:53 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, armbru, qemu-trivial, pbonzini, Gerd Hoffmann,
Alistair Francis, Peter Crosthwaite, Kevin Wolf, Max Reitz,
Edgar E. Iglesias, Peter Maydell, Alexander Graf, Jason Wang,
Subbaraya Sundeep, Stefan Berger, Dr. David Alan Gilbert,
Christian Borntraeger, Cornelia Huck, Richard Henderson,
Michael S. Tsirkin, Igor Mammedov, open list:Block layer core,
open list:Xilinx Zynq, open list:New World, open list:S390
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/7] chardev: Use goto/label instead of do/break/while(0)
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 3/7] chardev: Use goto/label instead of do/break/while(0) Eric Blake
@ 2017-12-03 14:20 ` Marc-André Lureau
0 siblings, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2017-12-03 14:20 UTC (permalink / raw)
To: Eric Blake; +Cc: QEMU, qemu trival, Paolo Bonzini, Markus Armbruster
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] chardev: Clean up previous patch indentation
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 4/7] chardev: Clean up previous patch indentation Eric Blake
@ 2017-12-03 14:20 ` Marc-André Lureau
0 siblings, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2017-12-03 14:20 UTC (permalink / raw)
To: Eric Blake; +Cc: QEMU, qemu trival, Paolo Bonzini, Markus Armbruster
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup
2017-12-01 23:24 [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
` (6 preceding siblings ...)
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 7/7] checkpatch: Enforce proper do/while (0) style Eric Blake
@ 2018-01-09 20:52 ` Eric Blake
2018-01-10 13:52 ` Paolo Bonzini
7 siblings, 1 reply; 13+ messages in thread
From: Eric Blake @ 2018-01-09 20:52 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, pbonzini, armbru
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup
2018-01-09 20:52 ` [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
@ 2018-01-10 13:52 ` Paolo Bonzini
0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2018-01-10 13:52 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: qemu-trivial, armbru
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(-)
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-01-10 13:52 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-01 23:24 [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 1/7] net: Drop unusual use of do { } while (0); Eric Blake
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 2/7] mips: Tweak location of ';' in macros Eric Blake
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 3/7] chardev: Use goto/label instead of do/break/while(0) Eric Blake
2017-12-03 14:20 ` Marc-André Lureau
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 4/7] chardev: Clean up previous patch indentation Eric Blake
2017-12-03 14:20 ` Marc-André Lureau
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 5/7] tests: Avoid 'do/while(false); ' in vhost-user-bridge Eric Blake
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 6/7] maint: Fix macros with broken 'do/while(0); ' usage Eric Blake
2017-12-02 11:53 ` Juan Quintela
2017-12-01 23:24 ` [Qemu-devel] [PATCH v2 7/7] checkpatch: Enforce proper do/while (0) style Eric Blake
2018-01-09 20:52 ` [Qemu-devel] [PATCH v2 0/7] macro do/while (0) cleanup Eric Blake
2018-01-10 13:52 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).