[PATCH 1/3] staging: rtl8188eu: wrap macro parameters in parentheses

Sebastian Haas sehaas at deebas.com
Tue Mar 7 23:49:17 UTC 2017


On Tue, Mar 07, 2017 at 11:06:48PM +0300, Dan Carpenter wrote:
> On Sat, Mar 04, 2017 at 11:05:58PM +0100, Sebastian Haas wrote:
> > diff --git a/drivers/staging/rtl8188eu/include/rtl8188e_hal.h b/drivers/staging/rtl8188eu/include/rtl8188e_hal.h
> > index 9330361..ca9db14 100644
> > --- a/drivers/staging/rtl8188eu/include/rtl8188e_hal.h
> > +++ b/drivers/staging/rtl8188eu/include/rtl8188e_hal.h
> > @@ -146,7 +146,7 @@ struct txpowerinfo24g {
> >  #define EFUSE_REAL_CONTENT_LEN		512
> >  #define EFUSE_MAX_SECTION		16
> >  #define EFUSE_IC_ID_OFFSET		506 /* For some inferior IC purpose*/
> > -#define AVAILABLE_EFUSE_ADDR(addr)	(addr < EFUSE_REAL_CONTENT_LEN)
> > +#define AVAILABLE_EFUSE_ADDR(addr)	((addr) < EFUSE_REAL_CONTENT_LEN)
> 
> This one is never used.  I feel like you're just randomly adding
> parenthesis instead of taking your time to think about things.
> 

Not randomly, but maybe following too blindly the checkpatch findings.

> > --- a/drivers/staging/rtl8188eu/include/rtw_ioctl.h
> > +++ b/drivers/staging/rtl8188eu/include/rtw_ioctl.h
> > @@ -59,7 +59,7 @@
> >  #define OID_MP_SEG4		0xFF011100
> >  
> >  #define DEBUG_OID(dbg, str)						\
> > -	if ((!dbg)) {							\
> > +	if ((!(dbg))) {							\
> 
> Ugh...
> 
> >  #define rtw_get_ips_mode_req(pwrctrlpriv) \
> > -	(pwrctrlpriv)->ips_mode_req
> > +	((pwrctrlpriv)->ips_mode_req)
> 
> Do you really think this is an issue?  I know I should be in favour of
> defensive coding and all that, but I really feel like the focus should
> be on real issues, cleaning things up and deleting as much of this code
> as we can instead of just adding parenthesis.

You are right, I have not checked if the code is still in use and worth 
to rewrite it. I'll try to come up with a better patch.

thanks,
  sebastian


More information about the devel mailing list