Messages in this thread | | | Subject | Re: [PATCH] parport: fix if-statement empty body warnings | From | Randy Dunlap <> | Date | Thu, 27 Feb 2020 18:52:19 -0800 |
| |
On 2/27/20 6:32 PM, Joe Perches wrote: > On Thu, 2020-02-27 at 18:08 -0800, Randy Dunlap wrote: >> From: Randy Dunlap <rdunlap@infradead.org> >> >> When debugging via DPRINTK() is not enabled, make the DPRINTK() >> macro be an empty do-while block. >> >> This fixes gcc warnings when -Wextra is set: >> >> ../drivers/parport/ieee1284.c:262:18: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] >> ../drivers/parport/ieee1284.c:285:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] >> ../drivers/parport/ieee1284.c:298:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] >> ../drivers/parport/ieee1284_ops.c:576:18: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] >> >> I have verified that there is no object code change (with gcc 7.5.0). >> >> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> >> Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com> >> Cc: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> >> Cc: linux-parport@lists.infradead.org >> --- >> drivers/parport/ieee1284.c | 2 +- >> drivers/parport/ieee1284_ops.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> --- linux-next-20200225.orig/drivers/parport/ieee1284.c >> +++ linux-next-20200225/drivers/parport/ieee1284.c >> @@ -34,7 +34,7 @@ >> #ifdef DEBUG >> #define DPRINTK(stuff...) printk (stuff) >> #else >> -#define DPRINTK(stuff...) >> +#define DPRINTK(stuff...) do {} while (0) > > It's frequently better to use noprintk
Hi Joe,
I can't find that one. Oh, it's no_printk.
> And perhaps this should just become > > #define DPRINTK pr_debug > > or perhaps a conversion of DPRINTK uses
I'm OK with any of those. If you would also fix ieee1284.c:
CC [M] drivers/parport/ieee1284.o ../drivers/parport/ieee1284.c: In function ‘parport_ieee1284_terminate’: ../drivers/parport/ieee1284.c:262:18: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] port->name); ^ ../drivers/parport/ieee1284.c:285:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] port->name); ^ ../drivers/parport/ieee1284.c:298:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] port->name); ^
you can add my ack to the patch: Acked-by: Randy Dunlap <rdunlap@infradead.org>
Thanks.
> --- > drivers/parport/ieee1284_ops.c | 67 +++++++++++++++--------------------------- > 1 file changed, 23 insertions(+), 44 deletions(-) > > diff --git a/drivers/parport/ieee1284_ops.c b/drivers/parport/ieee1284_ops.c > index 5d41dd..b1c9f51 100644 > --- a/drivers/parport/ieee1284_ops.c > +++ b/drivers/parport/ieee1284_ops.c > @@ -27,12 +27,6 @@ > #undef DEBUG /* Don't want a garbled console */ > #endif > > -#ifdef DEBUG > -#define DPRINTK(stuff...) printk (stuff) > -#else > -#define DPRINTK(stuff...) > -#endif > - > /*** * > * One-way data transfer functions. * > * ***/ > @@ -115,7 +109,7 @@ size_t parport_ieee1284_write_compat (struct parport *port, > if (signal_pending (current)) > break; > > - DPRINTK (KERN_DEBUG "%s: Timed out\n", port->name); > + pr_debug("%s: Timed out\n", port->name); > break; > > ready: > @@ -178,9 +172,8 @@ size_t parport_ieee1284_read_nibble (struct parport *port, > if (parport_wait_peripheral (port, > PARPORT_STATUS_ACK, 0)) { > /* Timeout -- no more data? */ > - DPRINTK (KERN_DEBUG > - "%s: Nibble timeout at event 9 (%d bytes)\n", > - port->name, i/2); > + pr_debug("%s: Nibble timeout at event 9 (%d bytes)\n", > + port->name, i / 2); > parport_frob_control (port, PARPORT_CONTROL_AUTOFD, 0); > break; > } > @@ -201,8 +194,7 @@ size_t parport_ieee1284_read_nibble (struct parport *port, > PARPORT_STATUS_ACK, > PARPORT_STATUS_ACK)) { > /* Timeout -- no more data? */ > - DPRINTK (KERN_DEBUG > - "%s: Nibble timeout at event 11\n", > + pr_debug("%s: Nibble timeout at event 11\n", > port->name); > break; > } > @@ -219,9 +211,8 @@ size_t parport_ieee1284_read_nibble (struct parport *port, > /* Read the last nibble without checking data avail. */ > if (parport_read_status (port) & PARPORT_STATUS_ERROR) { > end_of_data: > - DPRINTK (KERN_DEBUG > - "%s: No more nibble data (%d bytes)\n", > - port->name, i/2); > + pr_debug("%s: No more nibble data (%d bytes)\n", > + port->name, i / 2); > > /* Go to reverse idle phase. */ > parport_frob_control (port, > @@ -272,8 +263,7 @@ size_t parport_ieee1284_read_byte (struct parport *port, > /* Timeout -- no more data? */ > parport_frob_control (port, PARPORT_CONTROL_AUTOFD, > 0); > - DPRINTK (KERN_DEBUG "%s: Byte timeout at event 9\n", > - port->name); > + pr_debug("%s: Byte timeout at event 9\n", port->name); > break; > } > > @@ -288,8 +278,7 @@ size_t parport_ieee1284_read_byte (struct parport *port, > PARPORT_STATUS_ACK, > PARPORT_STATUS_ACK)) { > /* Timeout -- no more data? */ > - DPRINTK (KERN_DEBUG "%s: Byte timeout at event 11\n", > - port->name); > + pr_debug("%s: Byte timeout at event 11\n", port->name); > break; > } > > @@ -307,8 +296,7 @@ size_t parport_ieee1284_read_byte (struct parport *port, > /* Read the last byte without checking data avail. */ > if (parport_read_status (port) & PARPORT_STATUS_ERROR) { > end_of_data: > - DPRINTK (KERN_DEBUG > - "%s: No more byte data (%zd bytes)\n", > + pr_debug("%s: No more byte data (%zd bytes)\n", > port->name, count); > > /* Go to reverse idle phase. */ > @@ -353,12 +341,10 @@ int ecp_forward_to_reverse (struct parport *port) > PARPORT_STATUS_PAPEROUT, 0); > > if (!retval) { > - DPRINTK (KERN_DEBUG "%s: ECP direction: reverse\n", > - port->name); > + pr_debug("%s: ECP direction: reverse\n", port->name); > port->ieee1284.phase = IEEE1284_PH_REV_IDLE; > } else { > - DPRINTK (KERN_DEBUG "%s: ECP direction: failed to reverse\n", > - port->name); > + pr_debug("%s: ECP direction: failed to reverse\n", port->name); > port->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN; > } > > @@ -384,12 +370,10 @@ int ecp_reverse_to_forward (struct parport *port) > > if (!retval) { > parport_data_forward (port); > - DPRINTK (KERN_DEBUG "%s: ECP direction: forward\n", > - port->name); > + pr_debug("%s: ECP direction: forward\n", port->name); > port->ieee1284.phase = IEEE1284_PH_FWD_IDLE; > } else { > - DPRINTK (KERN_DEBUG > - "%s: ECP direction: failed to switch forward\n", > + pr_debug("%s: ECP direction: failed to switch forward\n", > port->name); > port->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN; > } > @@ -450,7 +434,7 @@ size_t parport_ieee1284_ecp_write_data (struct parport *port, > } > > /* Time for Host Transfer Recovery (page 41 of IEEE1284) */ > - DPRINTK (KERN_DEBUG "%s: ECP transfer stalled!\n", port->name); > + pr_debug("%s: ECP transfer stalled!\n", port->name); > > parport_frob_control (port, PARPORT_CONTROL_INIT, > PARPORT_CONTROL_INIT); > @@ -466,8 +450,7 @@ size_t parport_ieee1284_ecp_write_data (struct parport *port, > if (!(parport_read_status (port) & PARPORT_STATUS_PAPEROUT)) > break; > > - DPRINTK (KERN_DEBUG "%s: Host transfer recovered\n", > - port->name); > + pr_debug("%s: Host transfer recovered\n", port->name); > > if (time_after_eq (jiffies, expire)) break; > goto try_again; > @@ -565,23 +548,20 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port, > command or a normal data byte, don't accept it. */ > if (command) { > if (byte & 0x80) { > - DPRINTK (KERN_DEBUG "%s: stopping short at " > - "channel command (%02x)\n", > + pr_debug("%s: stopping short at channel command (%02x)\n", > port->name, byte); > goto out; > } > else if (port->ieee1284.mode != IEEE1284_MODE_ECPRLE) > - DPRINTK (KERN_DEBUG "%s: device illegally " > - "using RLE; accepting anyway\n", > + pr_debug("%s: device illegally using RLE; accepting anyway\n", > port->name); > > rle_count = byte + 1; > > /* Are we allowed to read that many bytes? */ > if (rle_count > (len - count)) { > - DPRINTK (KERN_DEBUG "%s: leaving %d RLE bytes " > - "for next time\n", port->name, > - rle_count); > + pr_debug("%s: leaving %d RLE bytes for next time\n", > + port->name, rle_count); > break; > } > > @@ -596,7 +576,7 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port, > PARPORT_STATUS_ACK)) { > /* It's gone wrong. Return what data we have > to the caller. */ > - DPRINTK (KERN_DEBUG "ECP read timed out at 45\n"); > + pr_debug("ECP read timed out at 45\n"); > > if (command) > printk (KERN_WARNING > @@ -620,7 +600,7 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port, > memset (buf, byte, rle_count); > buf += rle_count; > count += rle_count; > - DPRINTK (KERN_DEBUG "%s: decompressed to %d bytes\n", > + pr_debug("%s: decompressed to %d bytes\n", > port->name, rle_count); > } else { > /* Normal data byte. */ > @@ -686,7 +666,7 @@ size_t parport_ieee1284_ecp_write_addr (struct parport *port, > } > > /* Time for Host Transfer Recovery (page 41 of IEEE1284) */ > - DPRINTK (KERN_DEBUG "%s: ECP transfer stalled!\n", port->name); > + pr_debug("%s: ECP transfer stalled!\n", port->name); > > parport_frob_control (port, PARPORT_CONTROL_INIT, > PARPORT_CONTROL_INIT); > @@ -702,8 +682,7 @@ size_t parport_ieee1284_ecp_write_addr (struct parport *port, > if (!(parport_read_status (port) & PARPORT_STATUS_PAPEROUT)) > break; > > - DPRINTK (KERN_DEBUG "%s: Host transfer recovered\n", > - port->name); > + pr_debug("%s: Host transfer recovered\n", port->name); > > if (time_after_eq (jiffies, expire)) break; > goto try_again; > >
-- ~Randy Reported-by: Randy Dunlap <rdunlap@infradead.org>
| |