headers
Dan Carpenter | 18 Aug 2012 10:47
Picon
Favicon

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

"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 <at> 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
 <at>  <at>  -213,7 +213,7  <at>  <at>  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;
Permalink | Reply |
headers
Rupesh Gujare | 19 Aug 2012 14:15

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

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 <at> oracle.com>

Acked-by: Rupesh Gujare<rgujare <at> 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
>  <at>  <at>  -213,7 +213,7  <at>  <at>  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
Permalink | Reply |
headers
Tomas Winkler | 19 Aug 2012 14:37
Picon
Favicon

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

> 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 <at> oracle.com>
>
>
> Acked-by: Rupesh Gujare<rgujare <at> 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
>>  <at>  <at>  -213,7 +213,7  <at>  <at>  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;
(Continue reading)

Permalink | Reply |
headers
Rupesh Gujare | 19 Aug 2012 15:06

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

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 <at> oracle.com>
>>
>> Acked-by: Rupesh Gujare<rgujare <at> 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
Permalink | Reply |
headers
Tomas Winkler | 19 Aug 2012 18:20
Picon
Favicon

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

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

As network stack already provides helper function
is_zero_ether_addr() we use that instead of memcmp

Signed-off-by: Tomas Winkler <tomas.winkler <at> intel.com>
Cc: Dan Carpenter <dan.carpenter <at> oracle.com>
Cc: Rupesh Gujare <rgujare <at> ozmodevices.com>
---
V1: by Dan Carpenter <dan.carpenter <at> oracle.com>
V2: replace memcmp with is_zero_ether_addr

 drivers/staging/ozwpan/ozcdev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
index d983219..77c4104 100644
--- a/drivers/staging/ozwpan/ozcdev.c
+++ b/drivers/staging/ozwpan/ozcdev.c
 <at>  <at>  -213,7 +213,7  <at>  <at>  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 (is_zero_ether_addr(addr)) {
 			spin_lock_bh(&g_cdev.lock);
 			pd = g_cdev.active_pd;
 			g_cdev.active_pd = 0;
--

--
(Continue reading)

Permalink | Reply |
headers
Dan Carpenter | 4 Sep 2012 19:31
Picon
Favicon

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

On Sun, Aug 19, 2012 at 07:20:54PM +0300, Tomas Winkler wrote:
> "addr" is a pointer so it's either 4 or 8 bytes, but actually we want
> to compare 6 bytes (ETH_ALEN).
> 
> As network stack already provides helper function
> is_zero_ether_addr() we use that instead of memcmp
> 
> Signed-off-by: Tomas Winkler <tomas.winkler <at> intel.com>
> Cc: Dan Carpenter <dan.carpenter <at> oracle.com>
> Cc: Rupesh Gujare <rgujare <at> ozmodevices.com>
> ---

Generally I would frown on patch thievery but since the maintainer
asked you to then it's fine.

Reported-by: Dan Carpenter <dan.carpenter <at> oracle.com>
Acked-by: Dan Carpenter <dan.carpenter <at> oracle.com>

regards,
dan carpenter
Permalink | Reply |
headers
Dan Carpenter | 4 Sep 2012 19:33
Picon
Favicon

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

On Tue, Sep 04, 2012 at 10:31:05AM -0700, Dan Carpenter wrote:
> On Sun, Aug 19, 2012 at 07:20:54PM +0300, Tomas Winkler wrote:
> > "addr" is a pointer so it's either 4 or 8 bytes, but actually we want
> > to compare 6 bytes (ETH_ALEN).
> > 
> > As network stack already provides helper function
> > is_zero_ether_addr() we use that instead of memcmp
> > 
> > Signed-off-by: Tomas Winkler <tomas.winkler <at> intel.com>
> > Cc: Dan Carpenter <dan.carpenter <at> oracle.com>
> > Cc: Rupesh Gujare <rgujare <at> ozmodevices.com>
> > ---
> 
> Generally I would frown on patch thievery but since the maintainer
> asked you to then it's fine.
> 
> Reported-by: Dan Carpenter <dan.carpenter <at> oracle.com>
> Acked-by: Dan Carpenter <dan.carpenter <at> oracle.com>

Oops.  Doesn't compile.  I see you sent a v3 already.

regards,
dan carpenter

> 
> regards,
> dan carpenter
Permalink | Reply |
headers
Dan Carpenter | 20 Aug 2012 00:23
Picon

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

On 8/19/12, Rupesh Gujare <rgujare <at> 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 <at> oracle.com>
>>>
>>> Acked-by: Rupesh Gujare<rgujare <at> 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
Permalink | Reply |

Gmane