[patch] staging: ozwpan: fix memcmp() test in oz_set_active_pd()

[patch] staging: ozwpan: fix memcmp() test in oz_set_active_pd()

[Dan Carpenter]

"addr" is a pointer so it's either 4 or 8 bytes, but actually we want
to compare 6 bytes (ETH_ALEN).

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
index d983219..63c1b67 100644
--- a/drivers/staging/ozwpan/ozcdev.c
+++ b/drivers/staging/ozwpan/ozcdev.c
@@ -213,7 +213,7 @@ static int oz_set_active_pd(u8 *addr)
 		if (old_pd)
 			oz_pd_put(old_pd);
 	} else {
-		if (!memcmp(addr, "\0\0\0\0\0\0", sizeof(addr))) {
+		if (!memcmp(addr, "\0\0\0\0\0\0", ETH_ALEN)) {
 			spin_lock_bh(&g_cdev.lock);
 			pd = g_cdev.active_pd;
 			g_cdev.active_pd = 0;

Re: [patch] staging: ozwpan: fix memcmp() test in oz_set_active_pd()

[Rupesh Gujare]

On 18/08/12 09:47, Dan Carpenter wrote:
> "addr" is a pointer so it's either 4 or 8 bytes, but actually we want
> to compare 6 bytes (ETH_ALEN).
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Rupesh Gujare<rgujare@ozmodevices.com>


> diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
> index d983219..63c1b67 100644
> --- a/drivers/staging/ozwpan/ozcdev.c
> +++ b/drivers/staging/ozwpan/ozcdev.c
> @@ -213,7 +213,7 @@ static int oz_set_active_pd(u8 *addr)
>   		if (old_pd)
>   			oz_pd_put(old_pd);
>   	} else {
> -		if (!memcmp(addr, "\0\0\0\0\0\0", sizeof(addr))) {
> +		if (!memcmp(addr, "\0\0\0\0\0\0", ETH_ALEN)) {
>   			spin_lock_bh(&g_cdev.lock);
>   			pd = g_cdev.active_pd;
>   			g_cdev.active_pd = 0;
>


-- 
Regards,
Rupesh Gujare

Re: [patch] staging: ozwpan: fix memcmp() test in oz_set_active_pd()

[Tomas Winkler]

> On 18/08/12 09:47, Dan Carpenter wrote:
>>
>> "addr" is a pointer so it's either 4 or 8 bytes, but actually we want
>> to compare 6 bytes (ETH_ALEN).
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
>
> Acked-by: Rupesh Gujare<rgujare@ozmodevices.com>

why not to use 'if (is_zero_ether_addr())" from linux/etherdevice.h
Thanks
Tomas


>
>
>
>> diff --git a/drivers/staging/ozwpan/ozcdev.c
>> b/drivers/staging/ozwpan/ozcdev.c
>> index d983219..63c1b67 100644
>> --- a/drivers/staging/ozwpan/ozcdev.c
>> +++ b/drivers/staging/ozwpan/ozcdev.c
>> @@ -213,7 +213,7 @@ static int oz_set_active_pd(u8 *addr)
>>                 if (old_pd)
>>                         oz_pd_put(old_pd);
>>         } else {
>> -               if (!memcmp(addr, "\0\0\0\0\0\0", sizeof(addr))) {
>> +               if (!memcmp(addr, "\0\0\0\0\0\0", ETH_ALEN)) {
>>                         spin_lock_bh(&g_cdev.lock);
>>                         pd = g_cdev.active_pd;
>>                         g_cdev.active_pd = 0;
>>
>
>
> --
> Regards,
> Rupesh Gujare
>
>
>
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

Re: [patch] staging: ozwpan: fix memcmp() test in oz_set_active_pd()

[Rupesh Gujare]

On 19/08/12 13:37, Tomas Winkler wrote:
>> On 18/08/12 09:47, Dan Carpenter wrote:
>>> "addr" is a pointer so it's either 4 or 8 bytes, but actually we want
>>> to compare 6 bytes (ETH_ALEN).
>>>
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> Acked-by: Rupesh Gujare<rgujare@ozmodevices.com>
> why not to use 'if (is_zero_ether_addr())" from linux/etherdevice.h
>
Tom,

Thanks for pointing this out, that looks more efficient. Interested in 
submitting a patch ?

Dan,

What do you think ?


-- 
Regards,
Rupesh Gujare

Re: [patch] staging: ozwpan: fix memcmp() test in oz_set_active_pd()

[Dan Carpenter]

On 8/19/12, Rupesh Gujare <rgujare@ozmodevices.com> wrote:
> On 19/08/12 13:37, Tomas Winkler wrote:
>>> On 18/08/12 09:47, Dan Carpenter wrote:
>>>> "addr" is a pointer so it's either 4 or 8 bytes, but actually we want
>>>> to compare 6 bytes (ETH_ALEN).
>>>>
>>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>
>>> Acked-by: Rupesh Gujare<rgujare@ozmodevices.com>
>> why not to use 'if (is_zero_ether_addr())" from linux/etherdevice.h
>>
> Tom,
>
> Thanks for pointing this out, that looks more efficient. Interested in
> submitting a patch ?
>
> Dan,
>
> What do you think ?
>

Yes yes.  Of course.  It's the weekend and I'm traveling but I can redo this.

regards,
dan carpenter