Hello Dan.

On 2015년 08월 13일 23:49, Dan Carpenter wrote:
On Thu, Aug 13, 2015 at 01:41:23PM +0900, Tony Cho wrote:
+static u32 get_id_from_handler(tstrWILC_WFIDrv *handler)
+{
+       u32 id;
+
+       if (!handler)
+               return 0;
+
+       for (id = 0; id < NUM_CONCURRENT_IFC; id++) {
+               if (wfidrv_list[id] == handler) {
+                       id += 1;
+                       break;
+               }
+       }
+
+       if (id > NUM_CONCURRENT_IFC)
+               return 0;
+       else
+               return id;
+}
+
This still has an off by one bug.  Just use zero offset arrays
throughout.

static int get_id_from_handler(tstrWILC_WFIDrv *handler)
{
        int id;

        if (!handler)
                return -ENOBUFS;

        for (id = 0; id < NUM_CONCURRENT_IFC; id++) {
                if (wfidrv_list[id] == handler)
                        return id;
        }

        return -ENOBUFS;
}
Thanks for your review. The return value of this function has from 0 till 2.
1 and 2 value is real ID value. only 0 value is reserved to remove a registered id. But I also think that error handling should be added about the overflowed value
as your opinion.
+static tstrWILC_WFIDrv *get_handler_from_id(u32 id)
+{
+       if (id > 0 && id <= NUM_CONCURRENT_IFC)
+               return wfidrv_list[id - 1];
+       else
+               return NULL;
+}
static tstrWILC_WFIDrv *get_handler_from_id(int id)
{
        if (id < 0 || id >= NUM_CONCURRENT_IFC)
                return NULL;
        return wfidrv_list[id];
}

regards,
dan carpenter


Regards.
Johnny Kim.
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to