All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] staging: wlan-ng: replace memcpy with ether_addr_copy
@ 2015-10-02  9:59 Ioana Ciornei
  2015-10-02 10:34 ` [Outreachy kernel] " Sudip Mukherjee
  0 siblings, 1 reply; 3+ messages in thread
From: Ioana Ciornei @ 2015-10-02  9:59 UTC (permalink / raw)
  To: outreachy-kernel; +Cc: Ioana Ciornei

Replace memcpy with ether_addr_copy since the output of pahole
shows that the addresses are aligned.

struct p80211_hdr_a3 {
        __le16                     fc;                   /*     0     2 */
        u16                        dur;                  /*     2     2 */
        u8                         a1[6];                /*     4     6 */
        u8                         a2[6];                /*    10     6 */
        u8                         a3[6];                /*    16     6 */
        u16                        seq;                  /*    22     2 */

        /* size: 24, cachelines: 1, members: 6 */
        /* last cacheline: 24 bytes */
};

struct net_device {
        char                       name[16];             /*     0    16 */
        struct hlist_node          name_hlist;           /*    16    16 */

     ...............

 /* --- cacheline 12 boundary (768 bytes) --- */
        long unsigned int          last_rx;              /*   768     8 */
        unsigned char *            dev_addr;             /*   776     8 */
        struct netdev_rx_queue *   _rx;                  /*   784     8 */

    .................
};

struct wlandevice {
        struct wlandevice *        next;                 /*     0     8 */
    .................
        u8                         bssid[6];             /*   128     6 */
    .................
};

struct net_device {
        char                       name[16];             /*     0    16 */
    .................
        unsigned char *            dev_addr;             /*   776     8 */
    ................
};

struct wlan_ethhdr {
        u8                         daddr[6];             /*     0     6 */
        u8                         saddr[6];             /*     6     6 */
        u16                        type;                 /*    12     2 */

        /* size: 14, cachelines: 1, members: 3 */
        /* last cacheline: 14 bytes */
};

struct sockaddr {
        sa_family_t                sa_family;            /*     0     2 */
        char                       sa_data[14];          /*     2    14 */

        /* size: 16, cachelines: 1, members: 2 */
        /* last cacheline: 16 bytes */
};

struct p80211pstr6 {
        u8                         len;                  /*     0     1 */
        u8                         data[6];              /*     1     6 */

        /* size: 7, cachelines: 1, members: 2 */
        /* last cacheline: 7 bytes */
};

Coccinelle semantic patch:

@@
expression E1,E2;
@@

- memcpy(E1, E2, 6)
+ ether_addr_copy(E1, E2)

@@
expression E1,E2;
@@

- memcpy(E1, E2, ETH_ALEN)
+ ether_addr_copy(E1, E2)

Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
---
Changes in v2:
    - revised the cocci patch to be a more constrainting one
    - applied cocci patch on the entire wlan-ng folder
    - updated the patch description with further pahole output

Changes in v3:
    - make an explicit cast from u8 (*)[6] to const u8 * to avoid compile warning

 drivers/staging/wlan-ng/p80211conv.c   | 20 ++++++++++----------
 drivers/staging/wlan-ng/p80211netdev.c |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/wlan-ng/p80211conv.c b/drivers/staging/wlan-ng/p80211conv.c
index 1b02cdf..8b6de4f 100644
--- a/drivers/staging/wlan-ng/p80211conv.c
+++ b/drivers/staging/wlan-ng/p80211conv.c
@@ -178,21 +178,21 @@ 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, (const u8*)&e_hdr.daddr);
+		ether_addr_copy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr);
+		ether_addr_copy(p80211_hdr->a3.a3, wlandev->bssid);
 		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);
+		ether_addr_copy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr);
+		ether_addr_copy(p80211_hdr->a3.a3, (const u8*)&e_hdr.daddr);
 		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, (const u8*)&e_hdr.daddr);
+		ether_addr_copy(p80211_hdr->a3.a2, wlandev->bssid);
+		ether_addr_copy(p80211_hdr->a3.a3, (const u8*)&e_hdr.saddr);
 		break;
 	default:
 		netdev_err(wlandev->netdev,
@@ -243,7 +243,7 @@ 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);
+			ether_addr_copy(wlandev->spy_address[i], mac);
 			wlandev->spy_stat[i].level = rxmeta->signal;
 			wlandev->spy_stat[i].noise = rxmeta->noise;
 			wlandev->spy_stat[i].qual =
diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
index a9c1e0b..4cc6a00 100644
--- a/drivers/staging/wlan-ng/p80211netdev.c
+++ b/drivers/staging/wlan-ng/p80211netdev.c
@@ -644,7 +644,7 @@ static int p80211knetdev_set_mac_address(netdevice_t *dev, void *addr)
 	macaddr->status = P80211ENUM_msgitem_status_data_ok;
 	macaddr->len = sizeof(macaddr->data);
 	macaddr->data.len = ETH_ALEN;
-	memcpy(&macaddr->data.data, new_addr->sa_data, ETH_ALEN);
+	ether_addr_copy(&macaddr->data.data, new_addr->sa_data);
 
 	/* Set up the resultcode argument */
 	resultcode->did = DIDmsg_dot11req_mibset_resultcode;
-- 
2.1.4



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

* Re: [Outreachy kernel] [PATCH v3] staging: wlan-ng: replace memcpy with ether_addr_copy
  2015-10-02  9:59 [PATCH v3] staging: wlan-ng: replace memcpy with ether_addr_copy Ioana Ciornei
@ 2015-10-02 10:34 ` Sudip Mukherjee
  2015-10-02 10:53   ` Ioana Ciornei
  0 siblings, 1 reply; 3+ messages in thread
From: Sudip Mukherjee @ 2015-10-02 10:34 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: outreachy-kernel

On Fri, Oct 02, 2015 at 12:59:24PM +0300, Ioana Ciornei wrote:
> Replace memcpy with ether_addr_copy since the output of pahole
> shows that the addresses are aligned.

Please always check your patch using checkpatch before sending. And more
important than that your patch is introducing build warnings.

checkpatch errors:
ERROR: "(foo*)" should be "(foo *)"

Build warnings:

drivers/staging/wlan-ng/p80211netdev.c: In function ‘p80211knetdev_set_mac_address’:
drivers/staging/wlan-ng/p80211netdev.c:647:2: warning: passing argument 1 of ‘ether_addr_copy’ from incompatible pointer type [enabled by default]
  ether_addr_copy(&macaddr->data.data, new_addr->sa_data);
  ^
In file included from drivers/staging/wlan-ng/p80211netdev.c:65:0:
include/linux/etherdevice.h:271:20: note: expected ‘u8 *’ but argument is of type ‘u8 (*)[6]’
 static inline void ether_addr_copy(u8 *dst, const u8 *src)
                    ^

regards
sudip


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

* Re: [Outreachy kernel] [PATCH v3] staging: wlan-ng: replace memcpy with ether_addr_copy
  2015-10-02 10:34 ` [Outreachy kernel] " Sudip Mukherjee
@ 2015-10-02 10:53   ` Ioana Ciornei
  0 siblings, 0 replies; 3+ messages in thread
From: Ioana Ciornei @ 2015-10-02 10:53 UTC (permalink / raw)
  To: Sudip Mukherjee; +Cc: outreachy-kernel

On Fri, Oct 2, 2015 at 1:34 PM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
>
> On Fri, Oct 02, 2015 at 12:59:24PM +0300, Ioana Ciornei wrote:
> > Replace memcpy with ether_addr_copy since the output of pahole
> > shows that the addresses are aligned.
>
> Please always check your patch using checkpatch before sending. And more
> important than that your patch is introducing build warnings.
>
> checkpatch errors:
> ERROR: "(foo*)" should be "(foo *)"
>
> Build warnings:
>
> drivers/staging/wlan-ng/p80211netdev.c: In function ‘p80211knetdev_set_mac_address’:
> drivers/staging/wlan-ng/p80211netdev.c:647:2: warning: passing argument 1 of ‘ether_addr_copy’ from incompatible pointer type [enabled by default]
>   ether_addr_copy(&macaddr->data.data, new_addr->sa_data);
>   ^
> In file included from drivers/staging/wlan-ng/p80211netdev.c:65:0:
> include/linux/etherdevice.h:271:20: note: expected ‘u8 *’ but argument is of type ‘u8 (*)[6]’
>  static inline void ether_addr_copy(u8 *dst, const u8 *src)
>                     ^
>

Again, sorry.
I resent it.

Ioana


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

end of thread, other threads:[~2015-10-02 10:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-02  9:59 [PATCH v3] staging: wlan-ng: replace memcpy with ether_addr_copy Ioana Ciornei
2015-10-02 10:34 ` [Outreachy kernel] " Sudip Mukherjee
2015-10-02 10:53   ` Ioana Ciornei

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.