All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Drivers: staging: wlan-ng: p80211conv: replaced functions for copying/comparing ethernet addresses
@ 2015-09-29 22:16 Shivani Bhardwaj
  2015-09-30  1:57 ` [Outreachy kernel] " Greg KH
  2015-09-30  5:16 ` Julia Lawall
  0 siblings, 2 replies; 4+ messages in thread
From: Shivani Bhardwaj @ 2015-09-29 22:16 UTC (permalink / raw)
  To: outreachy-kernel

Replaced the function memcpy() by ether_addr_copy() and, memcmp() by
ether_addr_equal() as these functions are more preferable for copying
and comparing the ethernet addresses respectively.
Also fixed the issue of character limit of 80 per line and removed
one unnecessary statement.

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/wlan-ng/p80211conv.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/wlan-ng/p80211conv.c b/drivers/staging/wlan-ng/p80211conv.c
index 49f2ef8..9c4dba6 100644
--- a/drivers/staging/wlan-ng/p80211conv.c
+++ b/drivers/staging/wlan-ng/p80211conv.c
@@ -178,21 +178,23 @@ int skb_ether_to_p80211(wlandevice_t *wlandev, u32 ethconv,
 
 	switch (wlandev->macmode) {
 	case WLAN_MACMODE_IBSS_STA:
-		memcpy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a3, wlandev->bssid, ETH_ALEN);
+		ether_addr_copy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
+		ether_addr_copy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr,
+				ETH_ALEN);
+		ether_addr_copy(p80211_hdr->a3.a3, wlandev->bssid, ETH_ALEN);
 		break;
 	case WLAN_MACMODE_ESS_STA:
 		fc |= cpu_to_le16(WLAN_SET_FC_TODS(1));
-		memcpy(p80211_hdr->a3.a1, wlandev->bssid, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a3, &e_hdr.daddr, ETH_ALEN);
+		ether_addr_copy(p80211_hdr->a3.a1, wlandev->bssid, ETH_ALEN);
+		ether_addr_copy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr,
+				ETH_ALEN);
+		ether_addr_copy(p80211_hdr->a3.a3, &e_hdr.daddr, ETH_ALEN);
 		break;
 	case WLAN_MACMODE_ESS_AP:
 		fc |= cpu_to_le16(WLAN_SET_FC_FROMDS(1));
-		memcpy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a2, wlandev->bssid, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a3, &e_hdr.saddr, ETH_ALEN);
+		ether_addr_copy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
+		ether_addr_copy(p80211_hdr->a3.a2, wlandev->bssid, ETH_ALEN);
+		ether_addr_copy(p80211_hdr->a3.a3, &e_hdr.saddr, ETH_ALEN);
 		break;
 	default:
 		netdev_err(wlandev->netdev,
@@ -242,8 +244,8 @@ static void orinoco_spy_gather(wlandevice_t *wlandev, char *mac,
 
 	for (i = 0; i < wlandev->spy_number; i++) {
 
-		if (!memcmp(wlandev->spy_address[i], mac, ETH_ALEN)) {
-			memcpy(wlandev->spy_address[i], mac, ETH_ALEN);
+		if (!ether_addr_equal(wlandev->spy_address[i], mac, ETH_ALEN)) {
+			ether_addr_copy(wlandev->spy_address[i], mac, ETH_ALEN);
 			wlandev->spy_stat[i].level = rxmeta->signal;
 			wlandev->spy_stat[i].noise = rxmeta->noise;
 			wlandev->spy_stat[i].qual =
@@ -614,8 +616,6 @@ int p80211skb_rxmeta_attach(struct wlandevice *wlandev, struct sk_buff *skb)
 	rxmeta = kzalloc(sizeof(struct p80211_rxmeta), GFP_ATOMIC);
 
 	if (rxmeta == NULL) {
-		netdev_err(wlandev->netdev,
-			   "%s: Failed to allocate rxmeta.\n", wlandev->name);
 		result = 1;
 		goto exit;
 	}
-- 
2.1.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Outreachy kernel] [PATCH] Drivers: staging: wlan-ng: p80211conv: replaced functions for copying/comparing ethernet addresses
  2015-09-29 22:16 [PATCH] Drivers: staging: wlan-ng: p80211conv: replaced functions for copying/comparing ethernet addresses Shivani Bhardwaj
@ 2015-09-30  1:57 ` Greg KH
  2015-09-30  5:16 ` Julia Lawall
  1 sibling, 0 replies; 4+ messages in thread
From: Greg KH @ 2015-09-30  1:57 UTC (permalink / raw)
  To: Shivani Bhardwaj; +Cc: outreachy-kernel

On Wed, Sep 30, 2015 at 03:46:08AM +0530, Shivani Bhardwaj wrote:
> Replaced the function memcpy() by ether_addr_copy() and, memcmp() by
> ether_addr_equal() as these functions are more preferable for copying
> and comparing the ethernet addresses respectively.
> Also fixed the issue of character limit of 80 per line and removed
> one unnecessary statement.

When you start listing "other" things you did in a patch, that's a huge
sign that you need to split this up into different patches.  Remember,
it's "only one thing per patch".

Also, you can't just do a search/replace for ether_addr_equal(),
otherwise we would have done that a long time ago.  You have to be able
to prove that this function will work properly, and that depends on
knowing the layout of the structure to ensure that everything is aligned
properly.  Look at how other people have made this change in the
drivers/staging/ tree in the past for the type of proof that I need in
order to accept this change.

thanks,

greg k-h


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Outreachy kernel] [PATCH] Drivers: staging: wlan-ng: p80211conv: replaced functions for copying/comparing ethernet addresses
  2015-09-29 22:16 [PATCH] Drivers: staging: wlan-ng: p80211conv: replaced functions for copying/comparing ethernet addresses Shivani Bhardwaj
  2015-09-30  1:57 ` [Outreachy kernel] " Greg KH
@ 2015-09-30  5:16 ` Julia Lawall
       [not found]   ` <CAKHNQQFv07V3NZdJk3CQQBjmUgdr-3viHh-pG-r7vfHidTKkdg@mail.gmail.com>
  1 sibling, 1 reply; 4+ messages in thread
From: Julia Lawall @ 2015-09-30  5:16 UTC (permalink / raw)
  To: Shivani Bhardwaj; +Cc: outreachy-kernel

If you look at git log --oneline for this file, it seems that Staging: 
wlan-ng: is preferred for this file, rather than Drivers: staging: 
wlan-ng: p80211conv:.

On Wed, 30 Sep 2015, Shivani Bhardwaj wrote:

> Replaced the function memcpy() by ether_addr_copy() and, memcmp() by
> ether_addr_equal() as these functions are more preferable for copying
> and comparing the ethernet addresses respectively.
> Also fixed the issue of character limit of 80 per line and removed
> one unnecessary statement.

The removed statement is related to logging on kzalloc.  It has nothing to 
do with the other changes, so it should not be in the same patch.

julia

> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> ---
>  drivers/staging/wlan-ng/p80211conv.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/wlan-ng/p80211conv.c b/drivers/staging/wlan-ng/p80211conv.c
> index 49f2ef8..9c4dba6 100644
> --- a/drivers/staging/wlan-ng/p80211conv.c
> +++ b/drivers/staging/wlan-ng/p80211conv.c
> @@ -178,21 +178,23 @@ int skb_ether_to_p80211(wlandevice_t *wlandev, u32 ethconv,
>  
>  	switch (wlandev->macmode) {
>  	case WLAN_MACMODE_IBSS_STA:
> -		memcpy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
> -		memcpy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr, ETH_ALEN);
> -		memcpy(p80211_hdr->a3.a3, wlandev->bssid, ETH_ALEN);
> +		ether_addr_copy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
> +		ether_addr_copy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr,
> +				ETH_ALEN);
> +		ether_addr_copy(p80211_hdr->a3.a3, wlandev->bssid, ETH_ALEN);
>  		break;
>  	case WLAN_MACMODE_ESS_STA:
>  		fc |= cpu_to_le16(WLAN_SET_FC_TODS(1));
> -		memcpy(p80211_hdr->a3.a1, wlandev->bssid, ETH_ALEN);
> -		memcpy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr, ETH_ALEN);
> -		memcpy(p80211_hdr->a3.a3, &e_hdr.daddr, ETH_ALEN);
> +		ether_addr_copy(p80211_hdr->a3.a1, wlandev->bssid, ETH_ALEN);
> +		ether_addr_copy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr,
> +				ETH_ALEN);
> +		ether_addr_copy(p80211_hdr->a3.a3, &e_hdr.daddr, ETH_ALEN);
>  		break;
>  	case WLAN_MACMODE_ESS_AP:
>  		fc |= cpu_to_le16(WLAN_SET_FC_FROMDS(1));
> -		memcpy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
> -		memcpy(p80211_hdr->a3.a2, wlandev->bssid, ETH_ALEN);
> -		memcpy(p80211_hdr->a3.a3, &e_hdr.saddr, ETH_ALEN);
> +		ether_addr_copy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
> +		ether_addr_copy(p80211_hdr->a3.a2, wlandev->bssid, ETH_ALEN);
> +		ether_addr_copy(p80211_hdr->a3.a3, &e_hdr.saddr, ETH_ALEN);
>  		break;
>  	default:
>  		netdev_err(wlandev->netdev,
> @@ -242,8 +244,8 @@ static void orinoco_spy_gather(wlandevice_t *wlandev, char *mac,
>  
>  	for (i = 0; i < wlandev->spy_number; i++) {
>  
> -		if (!memcmp(wlandev->spy_address[i], mac, ETH_ALEN)) {
> -			memcpy(wlandev->spy_address[i], mac, ETH_ALEN);
> +		if (!ether_addr_equal(wlandev->spy_address[i], mac, ETH_ALEN)) {
> +			ether_addr_copy(wlandev->spy_address[i], mac, ETH_ALEN);
>  			wlandev->spy_stat[i].level = rxmeta->signal;
>  			wlandev->spy_stat[i].noise = rxmeta->noise;
>  			wlandev->spy_stat[i].qual =
> @@ -614,8 +616,6 @@ int p80211skb_rxmeta_attach(struct wlandevice *wlandev, struct sk_buff *skb)
>  	rxmeta = kzalloc(sizeof(struct p80211_rxmeta), GFP_ATOMIC);
>  
>  	if (rxmeta == NULL) {
> -		netdev_err(wlandev->netdev,
> -			   "%s: Failed to allocate rxmeta.\n", wlandev->name);
>  		result = 1;
>  		goto exit;
>  	}
> -- 
> 2.1.0
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20150929221607.GA15184%40ubuntu.
> For more options, visit https://groups.google.com/d/optout.
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Outreachy kernel] [PATCH] Drivers: staging: wlan-ng: p80211conv: replaced functions for copying/comparing ethernet addresses
       [not found]   ` <CAKHNQQFv07V3NZdJk3CQQBjmUgdr-3viHh-pG-r7vfHidTKkdg@mail.gmail.com>
@ 2015-09-30 18:07     ` Shivani Bhardwaj
  0 siblings, 0 replies; 4+ messages in thread
From: Shivani Bhardwaj @ 2015-09-30 18:07 UTC (permalink / raw)
  To: outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 5069 bytes --]

Thanks Greg, Julia! I'll keep in mind.

On Wed, Sep 30, 2015 at 11:36 PM, Shivani Bhardwaj <shivanib134@gmail.com>
wrote:

> Thanks Greg, Julia! I'll keep in mind.
>
> On Wed, Sep 30, 2015 at 10:46 AM, Julia Lawall <julia.lawall@lip6.fr>
> wrote:
>
>> If you look at git log --oneline for this file, it seems that Staging:
>> wlan-ng: is preferred for this file, rather than Drivers: staging:
>> wlan-ng: p80211conv:.
>>
>> On Wed, 30 Sep 2015, Shivani Bhardwaj wrote:
>>
>> > Replaced the function memcpy() by ether_addr_copy() and, memcmp() by
>> > ether_addr_equal() as these functions are more preferable for copying
>> > and comparing the ethernet addresses respectively.
>> > Also fixed the issue of character limit of 80 per line and removed
>> > one unnecessary statement.
>>
>> The removed statement is related to logging on kzalloc.  It has nothing to
>> do with the other changes, so it should not be in the same patch.
>>
>> julia
>>
>> > Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
>> > ---
>> >  drivers/staging/wlan-ng/p80211conv.c | 26 +++++++++++++-------------
>> >  1 file changed, 13 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/drivers/staging/wlan-ng/p80211conv.c
>> b/drivers/staging/wlan-ng/p80211conv.c
>> > index 49f2ef8..9c4dba6 100644
>> > --- a/drivers/staging/wlan-ng/p80211conv.c
>> > +++ b/drivers/staging/wlan-ng/p80211conv.c
>> > @@ -178,21 +178,23 @@ int skb_ether_to_p80211(wlandevice_t *wlandev,
>> u32 ethconv,
>> >
>> >       switch (wlandev->macmode) {
>> >       case WLAN_MACMODE_IBSS_STA:
>> > -             memcpy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
>> > -             memcpy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr,
>> ETH_ALEN);
>> > -             memcpy(p80211_hdr->a3.a3, wlandev->bssid, ETH_ALEN);
>> > +             ether_addr_copy(p80211_hdr->a3.a1, &e_hdr.daddr,
>> ETH_ALEN);
>> > +             ether_addr_copy(p80211_hdr->a3.a2,
>> wlandev->netdev->dev_addr,
>> > +                             ETH_ALEN);
>> > +             ether_addr_copy(p80211_hdr->a3.a3, wlandev->bssid,
>> ETH_ALEN);
>> >               break;
>> >       case WLAN_MACMODE_ESS_STA:
>> >               fc |= cpu_to_le16(WLAN_SET_FC_TODS(1));
>> > -             memcpy(p80211_hdr->a3.a1, wlandev->bssid, ETH_ALEN);
>> > -             memcpy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr,
>> ETH_ALEN);
>> > -             memcpy(p80211_hdr->a3.a3, &e_hdr.daddr, ETH_ALEN);
>> > +             ether_addr_copy(p80211_hdr->a3.a1, wlandev->bssid,
>> ETH_ALEN);
>> > +             ether_addr_copy(p80211_hdr->a3.a2,
>> wlandev->netdev->dev_addr,
>> > +                             ETH_ALEN);
>> > +             ether_addr_copy(p80211_hdr->a3.a3, &e_hdr.daddr,
>> ETH_ALEN);
>> >               break;
>> >       case WLAN_MACMODE_ESS_AP:
>> >               fc |= cpu_to_le16(WLAN_SET_FC_FROMDS(1));
>> > -             memcpy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
>> > -             memcpy(p80211_hdr->a3.a2, wlandev->bssid, ETH_ALEN);
>> > -             memcpy(p80211_hdr->a3.a3, &e_hdr.saddr, ETH_ALEN);
>> > +             ether_addr_copy(p80211_hdr->a3.a1, &e_hdr.daddr,
>> ETH_ALEN);
>> > +             ether_addr_copy(p80211_hdr->a3.a2, wlandev->bssid,
>> ETH_ALEN);
>> > +             ether_addr_copy(p80211_hdr->a3.a3, &e_hdr.saddr,
>> ETH_ALEN);
>> >               break;
>> >       default:
>> >               netdev_err(wlandev->netdev,
>> > @@ -242,8 +244,8 @@ static void orinoco_spy_gather(wlandevice_t
>> *wlandev, char *mac,
>> >
>> >       for (i = 0; i < wlandev->spy_number; i++) {
>> >
>> > -             if (!memcmp(wlandev->spy_address[i], mac, ETH_ALEN)) {
>> > -                     memcpy(wlandev->spy_address[i], mac, ETH_ALEN);
>> > +             if (!ether_addr_equal(wlandev->spy_address[i], mac,
>> ETH_ALEN)) {
>> > +                     ether_addr_copy(wlandev->spy_address[i], mac,
>> ETH_ALEN);
>> >                       wlandev->spy_stat[i].level = rxmeta->signal;
>> >                       wlandev->spy_stat[i].noise = rxmeta->noise;
>> >                       wlandev->spy_stat[i].qual =
>> > @@ -614,8 +616,6 @@ int p80211skb_rxmeta_attach(struct wlandevice
>> *wlandev, struct sk_buff *skb)
>> >       rxmeta = kzalloc(sizeof(struct p80211_rxmeta), GFP_ATOMIC);
>> >
>> >       if (rxmeta == NULL) {
>> > -             netdev_err(wlandev->netdev,
>> > -                        "%s: Failed to allocate rxmeta.\n",
>> wlandev->name);
>> >               result = 1;
>> >               goto exit;
>> >       }
>> > --
>> > 2.1.0
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> Groups "outreachy-kernel" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> > To post to this group, send email to outreachy-kernel@googlegroups.com.
>> > To view this discussion on the web visit
>> https://groups.google.com/d/msgid/outreachy-kernel/20150929221607.GA15184%40ubuntu
>> .
>> > For more options, visit https://groups.google.com/d/optout.
>> >
>>
>
>

[-- Attachment #2: Type: text/html, Size: 7098 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-09-30 18:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-29 22:16 [PATCH] Drivers: staging: wlan-ng: p80211conv: replaced functions for copying/comparing ethernet addresses Shivani Bhardwaj
2015-09-30  1:57 ` [Outreachy kernel] " Greg KH
2015-09-30  5:16 ` Julia Lawall
     [not found]   ` <CAKHNQQFv07V3NZdJk3CQQBjmUgdr-3viHh-pG-r7vfHidTKkdg@mail.gmail.com>
2015-09-30 18:07     ` Shivani Bhardwaj

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.