[PATCH 5/5] staging: wilc1000: use id value as argument
Johnny Kim
johnny.kim at atmel.com
Tue Aug 18 03:10:53 UTC 2015
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.
More information about the devel
mailing list