Re: [PATCH] staging: rtl8188eu: Macro should be enclosed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Apr 01, 2017 at 09:24:58AM +0200, Sebastian Haas wrote:
> Hi Alfonso,
> 
> On Wed, Mar 29, 2017 at 07:50:11PM +0100, alfonsolimaastor@xxxxxxxxx wrote:
> > diff --git a/drivers/staging/rtl8188eu/include/odm_debug.h b/drivers/staging/rtl8188eu/include/odm_debug.h
> > index 687ff3e..fd92f7e 100644
> > --- a/drivers/staging/rtl8188eu/include/odm_debug.h
> > +++ b/drivers/staging/rtl8188eu/include/odm_debug.h
> > @@ -86,11 +86,13 @@
> >  #endif
> >  
> >  #define ODM_RT_TRACE(pDM_Odm, comp, level, fmt)				\
> > -	if (((comp) & pDM_Odm->DebugComponents) &&			\
> > -	    (level <= pDM_Odm->DebugLevel)) {				\
> > -		pr_info("[ODM-8188E] ");				\
> > -		RT_PRINTK fmt;						\
> > -	}
> > +	do {								\
> > +		if (((comp) & pDM_Odm->DebugComponents) &&		\
> > +		    (level <= pDM_Odm->DebugLevel)) {			\
> > +			pr_info("[ODM-8188E] ");			\
> > +			RT_PRINTK fmt;					\
> > +		}							\
> > +	} while (0)
> 
> isn't the if-statement already a single block?
> I don't think the do-while adds any improvement.

I have investigated a little bit and found no reason why checkpatch
complains about this, so I reported a bug:
http://www.spinics.net/lists/kernel/msg2484676.html

It turns out the problem comes when you use that macro inside an if-else
statement like this:

if (foo())
	ODM_RT_TRACE(pDM_Odm, comp, level, fmt);
else
	do_smomething();

So, checkpatch's complain seems to be legit.

Regards,
Alfonso
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux