On Sun, Apr 09, 2017 at 02:47:45PM +0100, alfonsolimaastor@xxxxxxxxx wrote:
> 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
Thanks for pointing that out. I have missed that 'else' scenario.
So, checkpatch was right and the patch is fine.
But I have a question about your example:
Wouldn't the original macro called with a trailing semicolon lead to a compile error?
On the other hand, a while-wrapped macro without semicolon would also result in an error.
/sebastian
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
