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