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