On Tue, Aug 18, 2015 at 12:10:53PM +0900, Johnny Kim wrote:

Hello Dan.

On 2015년 08월 13일 23:49, Dan Carpenter wrote:

quoted

On Thu, Aug 13, 2015 at 01:41:23PM +0900, Tony Cho wrote:

quoted

+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.

I thought we had created "id" here in this patch so we don't have to
pass function pointers through a u32 value (which can't fit a 64 bit
pointer).  What do you mean it is a "real ID value"?  Is it there in
the hardware spec?

Anyway, this code is buggy and messy.  Please find a different way to
write it.

regards,
dan carpenter