[PATCH 6/9] staging: brcm80211: fix checkpatch error 'assignment in if condition'

Joe Perches joe at perches.com
Wed Sep 29 22:25:28 UTC 2010


On Wed, 2010-09-29 at 14:19 -0400, jason wrote:
> Henry Ptasinski wrote:
> > On Mon, Sep 27, 2010 at 01:37:18PM -0700, Jason Cooper wrote:
> >> @@ -1847,7 +1858,12 @@ dhd_add_if(dhd_info_t *dhd, int ifidx, void *handle, char *name,
> >>  	ASSERT(dhd && (ifidx < DHD_MAX_IFS));
> >>  
> >>  	ifp = dhd->iflist[ifidx];
> >> -	if (!ifp && !(ifp = MALLOC(dhd->pub.osh, sizeof(dhd_if_t)))) {
> >> +	if (!ifp) {
> >> +		DHD_ERROR(("%s: dhd->iflist[ifidx] null\n", __func__));
> >> +		return -EINVAL;
> >> +	}
> >> +	ifp = MALLOC(dhd->pub.osh, sizeof(dhd_if_t));
> >> +	if (!ifp) {
> >>  		DHD_ERROR(("%s: OOM - dhd_if_t\n", __func__));
> >>  		return -ENOMEM;
> >>  	}
> > 
> > I think you changed the logic here from AND to OR.  I believe this would
> > be more correct:
> > 
> > 	ifp = MALLOC(dhd->pub.osh, sizeof(dhd_if_t));
> > 	if (!(dhd->iflist[ifidx]) && (!ifp)) {
> > 		DHD_ERROR(("%s: OOM - dhd_if_t\n", __func__));
> > 		return -ENOMEM;
> > 	}
> > 
> 
> I was attempting to remove the checkpatch.pl error with as little
> interpretation as possible.

Not all checkpatch output needs to be fixed.

Sometimes the best change is no change at all.

The current code is straightforward and intelligible.
The proposed changes make it worse.

You might remove the kmalloc wrappers though.




More information about the devel mailing list