Search Linux Wireless

Re: [PATCH 5/5] staging: wilc1000: use id value as argument

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Dan.
On 2015년 08월 18일 18:12, Dan Carpenter wrote:
On Tue, Aug 18, 2015 at 12:10:53PM +0900, Johnny Kim wrote:
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.
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?
Real ID value means the value mapped to an alive NIC handler.
And when the driver transmits and receives some data frame with chipset,
the ID is used to distinguish the data frame's owner. Just like the driver,
chipset uses the appointed identifier. the data frame always includes the
identifier.
You know, current driver is using 32bit pointer address as the identifier.
So, this patch converts the address value to integer value. As mentioned
earlier, '0' value is the reserved value to terminate an alive NIC handler
and inform it to chipset.
Anyway, this code is buggy and messy.  Please find a different way to
write it.

Regards.
Johnny.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux