Re: [PATCH 3/3] staging: vt6656: code cleanup, resolved sparse finding

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

 



>> and use that no NULL style consistently but maybe that's just me.

Thanks for your comments, I agree with you on not using NULL at all as
it seems to be easier to read to me.
I wasn't sure about the style to use, but your feedback helped a lot.

I will fix this patch and resend it.

-- Andres

On Tue, May 4, 2010 at 9:02 PM, Joe Perches <joe@xxxxxxxxxxx> wrote:
>
> On Tue, 2010-05-04 at 20:40 -0300, Andres More wrote:
> > Cleared sparse warning 'Using plain integer as NULL pointer'
>
> Hi Andres.
>
> > diff --git a/drivers/staging/vt6656/wmgr.c b/drivers/staging/vt6656/wmgr.c
> > index 1824551..5af98e9 100644
> > --- a/drivers/staging/vt6656/wmgr.c
> > +++ b/drivers/staging/vt6656/wmgr.c
> > @@ -958,12 +958,12 @@ s_vMgrRxAssocResponse(
> >          sFrame.pBuf = (PBYTE)pRxPacket->p80211Header;
> >          // decode the frame
> >          vMgrDecodeAssocResponse(&sFrame);
> > -        if ((sFrame.pwCapInfo == 0) ||
> > -            (sFrame.pwStatus == 0) ||
> > -            (sFrame.pwAid == 0) ||
> > -            (sFrame.pSuppRates == 0)){
> > -            DBG_PORT80(0xCC);
> > -            return;
> > +     if ((sFrame.pwCapInfo == NULL)
> > +         || (sFrame.pwStatus == NULL)
> > +         || (sFrame.pwAid == NULL)
> > +         || (sFrame.pSuppRates == NULL)) {
> > +             DBG_PORT80(0xCC);
> > +             return;
>
> The more common kernel style is to place logical continuations
> at the end of line, not at the beginning of a new line.
>
> You've mixed some comparisons with and without NULL in this patch.
> I think it'd be better to not use NULL, just the logical test as:
>
>        if (!sFrame.pwCapInfo || !sFrame.pwStatus ||
>            !sFrame.pwAid || !sFrame.pSuppRates) {
>
> and use that no NULL style consistently but maybe that's just me.
>
> It would also be good to get rid of the Hungarian and CamelCase,
> but one patch at a time...
>
> > @@ -2152,9 +2152,9 @@ if(ChannelExceedZoneType(pDevice,byCurrChannel)==TRUE)
> >          if (bTSFLargeDiff)
> >              bUpdateTSF = TRUE;
> >
> > -        if ((pDevice->bEnablePSMode == TRUE) &&(sFrame.pTIM != 0)) {
> > +     if ((pDevice->bEnablePSMode == TRUE) && (sFrame.pTIM)) {
>
> One of the no NULL uses above.
>
> > @@ -4286,7 +4289,7 @@ s_vMgrRxProbeResponse(
> >  if(ChannelExceedZoneType(pDevice,byCurrChannel)==TRUE)
> >        return;
> >
> > -    if (sFrame.pERP != NULL) {
> > +    if (sFrame.pERP) {
> >          sERP.byERP = sFrame.pERP->byContext;
> >          sERP.bERPExist = TRUE;
> >      } else {
>
> Another no NULL here
>
> cheers, Joe
>



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