macro do/while (0) cleanup
Subsystems (3)
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 many of the bad macros were the victims of copy-and-paste from some other bad location (particularly given how many bit-rotten debug print macros were involved).
https://wiki.qemu.org/BiteSizedTasks#Bitrot_prevention is still left for someone else, for another day.
Eric Blake (3):
net: Drop unusual use of do { } while (0);
mips: Tweak location of ';' in macros
maint: Fix macros with broken 'do/while(0);' usage
tests/acpi-utils.h | 8 ++++---- ui/sdl_zoom_template.h | 8 ++++---- audio/paaudio.c | 4 ++-- hw/adc/stm32f2xx_adc.c | 2 +- hw/block/m25p80.c | 2 +- hw/char/cadence_uart.c | 2 +- hw/char/stm32f2xx_usart.c | 2 +- hw/display/cg3.c | 2 +- hw/display/dpcd.c | 2 +- hw/display/xlnx_dp.c | 2 +- hw/dma/pl330.c | 2 +- hw/dma/xlnx-zynq-devcfg.c | 2 +- hw/dma/xlnx_dpdma.c | 2 +- hw/i2c/i2c-ddc.c | 2 +- hw/misc/auxbus.c | 2 +- hw/misc/macio/mac_dbdma.c | 4 ++-- hw/misc/mmio_interface.c | 2 +- hw/misc/stm32f2xx_syscfg.c | 2 +- hw/misc/zynq_slcr.c | 2 +- hw/net/cadence_gem.c | 2 +- hw/net/pcnet.c | 20 ++++++++++---------- hw/ssi/mss-spi.c | 2 +- hw/ssi/stm32f2xx_spi.c | 2 +- hw/ssi/xilinx_spi.c | 2 +- hw/ssi/xilinx_spips.c | 2 +- hw/timer/a9gtimer.c | 2 +- hw/timer/cadence_ttc.c | 2 +- hw/timer/mss-timer.c | 2 +- hw/timer/stm32f2xx_timer.c | 2 +- hw/tpm/tpm_passthrough.c | 2 +- hw/tpm/tpm_tis.c | 2 +- migration/rdma.c | 2 +- target/arm/translate-a64.c | 2 +- target/mips/msa_helper.c | 34 ++++++++++++++++++---------------- target/s390x/kvm.c | 2 +- tests/tcg/test-mmap.c | 2 +- 36 files changed, 70 insertions(+), 68 deletions(-)
Signature · 1 line
2.14.3
PATCH 1/3 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 '{ ... }'.
--- hw/net/pcnet.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
hw/net/pcnet.c +10 -10
@@ -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
Re: [Qemu-devel] [PATCH 1/3] net: Drop unusual use of do { } while (0);
Quoted 10 lines · Eric Blake
For a couple of macros in
pcnet.c, we have to provide a new scope to avoid compiler warnings about declarations in the middle of a switch statement that aren't in a sub-scope. But use of 'do { ... } while (0);' merely to provide that new scope is arcane overkill, compared to just using '{ ... }'.Signed-off-by Eric Blake <[email protected]>--- hw/net/pcnet.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
PATCH 2/3 mips: Tweak location of ';' in macros
In reply to Eric BlakeIt 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 '}'.
--- target/mips/msa_helper.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-)
target/mips/msa_helper.c +18 -16
@@ -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
Re: [Qemu-devel] [PATCH 2/3] mips: Tweak location of ';' in macros
Quoted 6 lines · 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]>
Quoted 125 lines
--- target/mips/msa_helper.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-)target/mips/msa_helper.c +18
-16@@ -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
PATCH 3/3 maint: Fix macros with broken 'do/while(0); ' usage
In reply to Eric BlakeThe point of writing a macro embedded in a 'do { ... } while (0)' loop is so that the macro can be used as a drop-in statement with the caller supplying the trailing ';'. Although our coding style frowns on brace-less 'if':
if (cond)
statement;
else
something else;the use of do/while (0) in a macro is absolutely essential for the purpose of avoiding a syntax error on the 'else' - but it only works if there is no trailing ';' in the macro (as the ';' in the code calling the macro would then be a second statement and cause the 'else' to not pair to the 'if').
Many of the places touched in this code are examples of the ugly bit-rotting debug print statements; cleaning those up is left as a bite-sized task for another day.
Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
--- 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
Re: [Qemu-arm] [PATCH 3/3] maint: Fix macros with broken 'do/while(0); ' usage
Quoted 63 lines · On Thu, 30 Nov 2017 07:41:59 -0600 Eric Blake <[email protected]> wrote:
The point of writing a macro embedded in a 'do { ... } while (0)' loop is so that the macro can be used as a drop-in statement with the caller supplying the trailing ';'. Although our coding style frowns on brace-less 'if':
if (cond)statement; else something else;the use of do/while (0) in a macro is absolutely essential for the purpose of avoiding a syntax error on the 'else' - but it only works if there is no trailing ';' in the macro (as the ';' in the code calling the macro would then be a second statement and cause the 'else' to not pair to the 'if').
Many of the places touched in this code are examples of the ugly bit-rotting debug print statements; cleaning those up is left as a bite-sized task for another day.
Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
Signed-off-by Eric Blake <[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(-)target/s390x/kvm.c +0
-0
target/s390x/kvm.c line 64
Quoted 8 lines
target/s390x/kvm.c +1
-1@@ -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)
Re: [Qemu-arm] [PATCH 3/3] maint: Fix macros with broken 'do/while(0); ' usage
In reply to Eric BlakeQuoted 5 lines · Eric Blake
The point of writing a macro embedded in a 'do { ... } while (0)' loop is so that the macro can be used as a drop-in statement with the caller supplying the trailing ';'. Although our coding style frowns on brace-less 'if':
if (cond)
Quoted 8 lines
statement; else something else;the use of do/while (0) in a macro is absolutely essential for the purpose of avoiding a syntax error on the 'else' - but it only works if there is no trailing ';' in the macro (as the ';' in the code calling the macro would then be a second statement and cause the 'else' to not pair to the 'if').
Shouldn't matter if everyone puts the statements in {}, right?
Quoted 7 lines
Many of the places touched in this code are examples of the ugly bit-rotting debug print statements; cleaning those up is left as a bite-sized task for another day.
Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
Signed-off-by Eric Blake <[email protected]>
We can't really rely on code still building for this to do the right thing, can we? I did my best to look for uses and I think it's OK, so
but I'm not merging this.
Quoted 2 lines
Signature · 1 line
2.14.3
Re: [Qemu-arm] [PATCH 3/3] maint: Fix macros with broken 'do/while(0); ' usage
Quoted 15 lines · Michael S. Tsirkin
Quoted 8 lines · Eric Blake
The point of writing a macro embedded in a 'do { ... } while (0)' loop is so that the macro can be used as a drop-in statement with the caller supplying the trailing ';'. Although our coding style frowns on brace-less 'if':
if (cond) statement; else something else;Quoted 5 lines
the use of do/while (0) in a macro is absolutely essential for the purpose of avoiding a syntax error on the 'else' - but it only works if there is no trailing ';' in the macro (as the ';' in the code calling the macro would then be a second statement and cause the 'else' to not pair to the 'if').
Quoted 1 line
Shouldn't matter if everyone puts the statements in {}, right?
Correct - where we follow our style, spurious ';' don't make a difference (other than they might trigger a warning in a very particular compiler). But it also makes our code harder to copy-and-paste into other projects with a different style.
Quoted 11 lines
Quoted 8 lines
Many of the places touched in this code are examples of the ugly bit-rotting debug print statements; cleaning those up is left as a bite-sized task for another day.
Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
Signed-off-by Eric Blake <[email protected]>We can't really rely on code still building for this to do the right thing, can we?
Some of the uses that were changed are in dead #ifdefs for debugging purposes - the only way to still compile is to turn on the debugging, but that may fail to compile for other reasons (if the format strings have bit-rotted, for example).
The only sure way to know that this did not break anything is to audit that for every macro where I eliminated the ';', all callers of that macro call 'foo();'. I did not perform that audit (the patch was mechanical) - but am willing to do so and reply back with results if you need the extra confidence.
Quoted 5 lines
I did my best to look for uses and I think it's OK, so
Reviewed-by Michael S. Tsirkin <[email protected]>but I'm not merging this.
What tree should it go through instead? Does it need to be split along maintainer boundaries?
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-arm] [PATCH 3/3] maint: Fix macros with broken 'do/while(0); ' usage
In reply to Eric BlakeQuoted 28 lines · * Eric Blake ([email protected]) wrote:
The point of writing a macro embedded in a 'do { ... } while (0)' loop is so that the macro can be used as a drop-in statement with the caller supplying the trailing ';'. Although our coding style frowns on brace-less 'if':
if (cond)statement; else something else;the use of do/while (0) in a macro is absolutely essential for the purpose of avoiding a syntax error on the 'else' - but it only works if there is no trailing ';' in the macro (as the ';' in the code calling the macro would then be a second statement and cause the 'else' to not pair to the 'if').
Many of the places touched in this code are examples of the ugly bit-rotting debug print statements; cleaning those up is left as a bite-sized task for another day.
Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
Signed-off-by Eric Blake <[email protected]>---
migration/rdma.c +0
-0
migration/rdma.c static uint32_t known_capabilities = RDMA_CAPABILITY_PIN_ALL; · line 91
Quoted 5 lines
migration/rdma.c +1
-1@@ -88,7 +88,7 @@ static uint32_t known_capabilities = RDMA_CAPABILITY_PIN_ALL; } \return rdma->error_state; \ } \ - }while (0 ); + }while (0 )
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-arm] [Qemu-ppc] [PATCH 3/3] maint: Fix macros with broken 'do/while(0); ' usage
In reply to Eric BlakeQuoted 14 lines · Eric Blake
The point of writing a macro embedded in a 'do { ... } while (0)' loop is so that the macro can be used as a drop-in statement with the caller supplying the trailing ';'. Although our coding style frowns on brace-less 'if':
if (cond)statement; else something else;the use of do/while (0) in a macro is absolutely essential for the purpose of avoiding a syntax error on the 'else' - but it only works if there is no trailing ';' in the macro (as the ';' in the code calling the macro would then be a second statement and cause the 'else' to not pair to the 'if').
Quoted 7 lines
Many of the places touched in this code are examples of the ugly bit-rotting debug print statements; cleaning those up is left as a bite-sized task for another day.
Found mechanically via: $ git grep -B1 'while (0);' | grep -A1 \\\\
Signed-off-by Eric Blake <[email protected]>
ppc related portions
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
[Qemu-devel] [PATCH 4/3] checkpatch: Enforce proper do/while (0) style
In reply to Eric Blakewhile (0) is only idiomatic in a macro definition, where the caller will be supplying the trailing ';'. Warn if the macro has a duplicate.
--- scripts/checkpatch.pl | 5 +++++ 1 file changed, 5 insertions(+)
scripts/checkpatch.pl +5 -0
@@ -1622,6 +1622,11 @@ sub process { } } +# 'while (0);' is odd; only macros should use while (0), without trailing ; + if ($line =~ /while\s*\(0\);/) { + ERROR("suspicious ; after while (0)\n" . $herecurr); + } + # Check relative indent for conditionals and blocks. if ($line =~ /\b(?:(?:if|while|for)\s*\(|do\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) { my ($s, $c) = ($stat, $cond);
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH 4/3] checkpatch: Enforce proper do/while (0) style
Quoted 13 lines · Eric Blake
while (0) is only idiomatic in a macro definition, where the caller will be supplying the trailing ';'. Warn if the macro has a duplicate.
Signed-off-by Eric Blake <[email protected]>--- scripts/checkpatch.pl | 5 +++++ 1 file changed, 5 insertions(+)scripts/checkpatch.pl +0
-0
scripts/checkpatch.pl sub process { · line 1626
Quoted 5 lines
scripts/checkpatch.pl +2
-0@@ -1622,6 +1622,11 @@ sub process { } } +# 'while (0);' is odd; only macros should use while (0), without trailing ; + if ($line =~ /while\s*\(0\);/) {
Should this also check for uses of 'while (false);' ?
Interestingly enough, we have an instance of 'do/while (false);' in tests/vhost-user-bridge.c that is NOT in a macro, but is used for the convenience of being able to 'break;' out early rather than using a goto. Similarly for chardev/char-serial.c using 'while (0);' outside of a macro. Those may be worth rewriting to use goto as separate patches if we want to restrict ALL use of 'while \((0|false)\);'
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 4/3] checkpatch: Enforce proper do/while (0) style
Quoted 17 lines · Eric Blake
Quoted 16 lines · Eric Blake
while (0) is only idiomatic in a macro definition, where the caller will be supplying the trailing ';'. Warn if the macro has a duplicate.
Signed-off-by Eric Blake <[email protected]>--- scripts/checkpatch.pl | 5 +++++ 1 file changed, 5 insertions(+)scripts/checkpatch.pl +0
-0@@ -1622,6 +1622,11 @@ sub process { } }
Quoted 4 lines
scripts/checkpatch.pl sub process { · line 1626
Quoted 2 lines
scripts/checkpatch.pl +2
-0+# 'while (0);' is odd; only macros should use while (0), without trailing ; + if ($line =~ /while\s*\(0\);/) {Should this also check for uses of 'while (false);' ?
Do we think it's likely to occur?
Quoted 5 lines
Interestingly enough, we have an instance of 'do/while (false);' in
tests/vhost-user-bridge.cthat is NOT in a macro, but is used for the convenience of being able to 'break;' out early rather than using a goto. Similarly forchardev/char-serial.cusing 'while (0);' outside of a macro.
That "cure" merely adds gratuitous cleverness to the "disease".
Quoted 2 lines
Those may be worth rewriting to use goto as separate
patches if we want to restrict ALL use of 'while \((0|false)\);'
I'd support that.
Re: [Qemu-devel] [PATCH 4/3] checkpatch: Enforce proper do/while (0) style
Quoted 8 lines · Markus Armbruster
Quoted 6 lines · Eric Blake
Quoted 5 lines · Eric Blake
while (0) is only idiomatic in a macro definition, where the caller will be supplying the trailing ';'. Warn if the macro has a duplicate.
Quoted 6 lines
Quoted 4 lines
scripts/checkpatch.pl sub process { · line 1626
Quoted 2 lines
scripts/checkpatch.pl +2
-0+# 'while (0);' is odd; only macros should use while (0), without trailing ; + if ($line =~ /while\s*\(0\);/) {Should this also check for uses of 'while (false);' ?
Do we think it's likely to occur?
Not as frequent, but it does appear in the code base.
Quoted 7 lines
Quoted 6 lines
Interestingly enough, we have an instance of 'do/while (false);' in
tests/vhost-user-bridge.cthat is NOT in a macro, but is used for the convenience of being able to 'break;' out early rather than using a goto. Similarly forchardev/char-serial.cusing 'while (0);' outside of a macro.
Quoted 7 lines
That "cure" merely adds gratuitous cleverness to the "disease".
Quoted 2 lines
Those may be worth rewriting to use goto as separate
patches if we want to restrict ALL use of 'while \((0|false)\);'I'd support that.
Okay, I'll post a v2 along those lines.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org