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
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to