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. _______________________________________________ devel mailing list [email protected] http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
