Joe,
On 09/29/2010 06:25 PM, Joe Perches wrote:
> 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.
>
Agreed.
> The current code is straightforward and intelligible.
> The proposed changes make it worse.
>
Ack. I'll revert to original when I resubmit.
> You might remove the kmalloc wrappers though.
>
That is already planned for a separate patch series. ;-)
thx,
Jason.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
