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

[jason]

Henry,

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.  The current code executes the MALLOC() if and only if ifp != NULL.  eg, if and only if dhd->iflist[ifidx] != NULL.

Do you really want to bail only when _both_ are NULL?  What if the MALLOC() failed?  Or, what if the dhd->iflist[ifidx] was NULL?


thx,

Jason.