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

Joe Perches joe at perches.com
Wed May 5 00:02:53 UTC 2010


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




More information about the devel mailing list