Skip to contentSkip to conversation

Replace memcpy with ether_addr_copy and fix checkpatch warning

PATCH Superseded Darshana Padmadas 4 participants 14 messages Mar 16, 2015 – Mar 18, 2015 Staging

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover Replace memcpy with ether_addr_copy and fix checkpatch warning

This patch set replaces memcpy with ether_addr_copy if the addresses of the structures are aligned. Structure layout is shown by using the Pahole tool.

This patch set fixes the following warning reported by checkpatch:
WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)

Darshana Padmadas (5):
Staging: rtl8192e: Replace memcpy with ether_addr_copy
Staging: rtl8192e: Use ether_addr_copy instead of memcpy
Staging: rtl8192e: Use ether_addr_copy replacing memcpy
Staging: rtl8192e: Use ether_addr_copy for aligned addresses
Staging: rtl8192e: replace memcpy with ether_addr_copy for aligned
structures

 drivers/staging/rtl8192e/rtl819x_BAProc.c    |  4 ++--
 drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 10 ++++----
 drivers/staging/rtl8192e/rtllib_rx.c         | 34 ++++++++++++++--------------
 drivers/staging/rtl8192e/rtllib_softmac.c    | 12 +++++-----
 drivers/staging/rtl8192e/rtllib_tx.c         |  6 ++---
 5 files changed, 33 insertions(+), 33 deletions(-)
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/5 Staging: rtl8192e: Replace memcpy with ether_addr_copy

This patch fixes the following warning found by checkpatch.pl:
Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2

Pahole showed that the structure for BAReq and Delba is aligned:
struct rtllib_hdr_3addr {

	__le16                     frame_ctl;            /*     0     2 */
	__le16                     duration_id;          /*     2     2 */
	u8                         addr1[6];             /*     4     6 */
	u8                         addr2[6];             /*    10     6 */
	u8                         addr3[6];             /*    16     6 */
	__le16                     seq_ctl;              /*    22     2 */
	u8                         payload[0];           /*    24     0 */
	/* size: 24, cachelines: 1, members: 7 */
	/* last cacheline: 24 bytes */
};
---
 drivers/staging/rtl8192e/rtl819x_BAProc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/rtl8192e/rtl819x_BAProc.c +2 -2
index 0415e02..9bab409 100644
@@ -103,7 +103,7 @@ static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst,
 	BAReq = (struct rtllib_hdr_3addr *)skb_put(skb,
 		 sizeof(struct rtllib_hdr_3addr));
 
-	memcpy(BAReq->addr1, Dst, ETH_ALEN);
+	ether_addr_copy(BAReq->addr1, Dst);
 	memcpy(BAReq->addr2, ieee->dev->dev_addr, ETH_ALEN);
 
 	memcpy(BAReq->addr3, ieee->current_network.bssid, ETH_ALEN);
@@ -168,7 +168,7 @@ static struct sk_buff *rtllib_DELBA(struct rtllib_device *ieee, u8 *dst,
 	Delba = (struct rtllib_hdr_3addr *) skb_put(skb,
 		 sizeof(struct rtllib_hdr_3addr));
 
-	memcpy(Delba->addr1, dst, ETH_ALEN);
+	ether_addr_copy(Delba->addr1, dst);
 	memcpy(Delba->addr2, ieee->dev->dev_addr, ETH_ALEN);
 	memcpy(Delba->addr3, ieee->current_network.bssid, ETH_ALEN);
 	Delba->frame_ctl = cpu_to_le16(RTLLIB_STYPE_MANAGE_ACT);
Signature · 1 line
1.9.1
#4 lore raw

PATCH 2/5 Staging: rtl8192e: Use ether_addr_copy instead of memcpy

In reply to Darshana Padmadas

This patch replaces use of memcpy with ether_addr_copy since the addresses for struct of hdr11 are aligned as shown by Pahole and hdr is of type u8.

struct rtllib_hdr_4addr {

	__le16                     frame_ctl;            /*     0     2 */
	__le16                     duration_id;          /*     2     2 */
	u8                         addr1[6];             /*     4     6 */
	u8                         addr2[6];             /*    10     6 */
	u8                         addr3[6];             /*    16     6 */
	__le16                     seq_ctl;              /*    22     2 */
	u8                         addr4[6];             /*    24     6 */
	u8                         payload[0];           /*    30     0 */
	/* size: 30, cachelines: 1, members: 8 */
	/* last cacheline: 30 bytes */
};

The following checkpatch warning is eliminated:
WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)

---
 drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/staging/rtl8192e/rtllib_crypt_tkip.c +5 -5
index 78db2b6..68a7fa6 100644
@@ -525,19 +525,19 @@ static void michael_mic_hdr(struct sk_buff *skb, u8 *hdr)
 	switch (le16_to_cpu(hdr11->frame_ctl) &
 		(RTLLIB_FCTL_FROMDS | RTLLIB_FCTL_TODS)) {
 	case RTLLIB_FCTL_TODS:
-		memcpy(hdr, hdr11->addr3, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr3); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN); /* SA */
 		break;
 	case RTLLIB_FCTL_FROMDS:
-		memcpy(hdr, hdr11->addr1, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr1); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr3, ETH_ALEN); /* SA */
 		break;
 	case RTLLIB_FCTL_FROMDS | RTLLIB_FCTL_TODS:
-		memcpy(hdr, hdr11->addr3, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr3); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr4, ETH_ALEN); /* SA */
 		break;
 	case 0:
-		memcpy(hdr, hdr11->addr1, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr1); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN); /* SA */
 		break;
 	}
@@ -591,7 +591,7 @@ static void rtllib_michael_mic_failure(struct net_device *dev,
 	else
 		ev.flags |= IW_MICFAILURE_PAIRWISE;
 	ev.src_addr.sa_family = ARPHRD_ETHER;
-	memcpy(ev.src_addr.sa_data, hdr->addr2, ETH_ALEN);
+	ether_addr_copy(ev.src_addr.sa_data, hdr->addr2);
 	memset(&wrqu, 0, sizeof(wrqu));
 	wrqu.data.length = sizeof(ev);
 	wireless_send_event(dev, IWEVMICHAELMICFAILURE, &wrqu, (char *) &ev);
Signature · 1 line
1.9.1
#7 lore raw

PATCH 3/5 Staging: rtl8192e: Use ether_addr_copy replacing memcpy

In reply to Darshana Padmadas

This patch uses ether_addr_copy instead of memcpy since the structures of beacon_buf, assoc, auth, a, header are aligned as shown by Pahole:

struct rtllib_probe_response {

	struct rtllib_hdr_3addr    header;               /*     0    24 */
	u32                        time_stamp[2];        /*    24     8 */
	__le16                     beacon_interval;      /*    32     2 */
	__le16                     capability;           /*    34     2 */
	struct rtllib_info_element info_element[0];      /*    36     0 */
	/* size: 36, cachelines: 1, members: 5 */
	/* last cacheline: 36 bytes */
};
struct rtllib_assoc_response_frame {
	struct rtllib_hdr_3addr    header;               /*     0    24 */
	__le16                     capability;           /*    24     2 */
	__le16                     status;               /*    26     2 */
	__le16                     aid;                  /*    28     2 */
	struct rtllib_info_element info_element[0];      /*    30     0 */
	/* size: 30, cachelines: 1, members: 5 */
	/* last cacheline: 30 bytes */
};
struct rtllib_authentication {
	struct rtllib_hdr_3addr    header;               /*     0    24 */
	__le16                     algorithm;            /*    24     2 */
	__le16                     transaction;          /*    26     2 */
	__le16                     status;               /*    28     2 */
	struct rtllib_info_element info_element[0];      /*    30     0 */
	/* size: 30, cachelines: 1, members: 5 */
	/* last cacheline: 30 bytes */
};
struct rtllib_hdr_3addr {
	__le16                     frame_ctl;            /*     0     2 */
	__le16                     duration_id;          /*     2     2 */
	u8                         addr1[6];             /*     4     6 */
	u8                         addr2[6];             /*    10     6 */
	u8                         addr3[6];             /*    16     6 */
	__le16                     seq_ctl;              /*    22     2 */
	u8                         payload[0];           /*    24     0 */
	/* size: 24, cachelines: 1, members: 7 */
	/* last cacheline: 24 bytes */
};

this eliminates the following warning found by checkpatch.pl:
WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)

---
 drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/staging/rtl8192e/rtllib_softmac.c +6 -6
index 16aef7c..d2ea955 100644
@@ -911,7 +911,7 @@ static struct sk_buff *rtllib_probe_resp(struct rtllib_device *ieee, u8 *dest)
 
 	beacon_buf = (struct rtllib_probe_response *) skb_put(skb,
 		     (beacon_size - ieee->tx_headroom));
-	memcpy(beacon_buf->header.addr1, dest, ETH_ALEN);
+	ether_addr_copy(beacon_buf->header.addr1, dest);
 	memcpy(beacon_buf->header.addr2, ieee->dev->dev_addr, ETH_ALEN);
 	memcpy(beacon_buf->header.addr3, ieee->current_network.bssid, ETH_ALEN);
 
@@ -1008,7 +1008,7 @@ static struct sk_buff *rtllib_assoc_resp(struct rtllib_device *ieee, u8 *dest)
 		skb_put(skb, sizeof(struct rtllib_assoc_response_frame));
 
 	assoc->header.frame_ctl = cpu_to_le16(RTLLIB_STYPE_ASSOC_RESP);
-	memcpy(assoc->header.addr1, dest, ETH_ALEN);
+	ether_addr_copy(assoc->header.addr1, dest);
 	memcpy(assoc->header.addr3, ieee->dev->dev_addr, ETH_ALEN);
 	memcpy(assoc->header.addr2, ieee->dev->dev_addr, ETH_ALEN);
 	assoc->capability = cpu_to_le16(ieee->iw_mode == IW_MODE_MASTER ?
@@ -1067,7 +1067,7 @@ static struct sk_buff *rtllib_auth_resp(struct rtllib_device *ieee, int status,
 
 	memcpy(auth->header.addr3, ieee->dev->dev_addr, ETH_ALEN);
 	memcpy(auth->header.addr2, ieee->dev->dev_addr, ETH_ALEN);
-	memcpy(auth->header.addr1, dest, ETH_ALEN);
+	ether_addr_copy(auth->header.addr1, dest);
 	auth->header.frame_ctl = cpu_to_le16(RTLLIB_STYPE_AUTH);
 	return skb;
 
@@ -1831,7 +1831,7 @@ static int auth_rq_parse(struct sk_buff *skb, u8 *dest)
 	}
 	a = (struct rtllib_authentication *) skb->data;
 
-	memcpy(dest, a->header.addr2, ETH_ALEN);
+	ether_addr_copy(dest, a->header.addr2);
 
 	if (le16_to_cpu(a->algorithm) != WLAN_AUTH_OPEN)
 		return  WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG;
@@ -1859,7 +1859,7 @@ static short probe_rq_parse(struct rtllib_device *ieee, struct sk_buff *skb,
 	if (bssid_match)
 		return -1;
 
-	memcpy(src, header->addr2, ETH_ALEN);
+	ether_addr_copy(src, header->addr2);
 
 	skbend = (u8 *)skb->data + skb->len;
 
@@ -1898,7 +1898,7 @@ static int assoc_rq_parse(struct sk_buff *skb, u8 *dest)
 
 	a = (struct rtllib_assoc_request_frame *) skb->data;
 
-	memcpy(dest, a->header.addr2, ETH_ALEN);
+	ether_addr_copy(dest, a->header.addr2);
 
 	return 0;
 }
Signature · 1 line
1.9.1
#8 lore raw

PATCH 4/5 Staging: rtl8192e: Use ether_addr_copy for aligned addresses

In reply to Darshana Padmadas

This patch replaces memcpy with ether_addr_copy since structures are aligned as shown by pahole:
struct rtllib_hdr_3addrqos {

	__le16                     frame_ctl;            /*     0     2 */
	__le16                     duration_id;          /*     2     2 */
	u8                         addr1[6];             /*     4     6 */
	u8                         addr2[6];             /*    10     6 */
	u8                         addr3[6];             /*    16     6 */
	__le16                     seq_ctl;              /*    22     2 */
	__le16                     qos_ctl;              /*    24     2 */
	u8                         payload[0];           /*    26     0 */
	/* size: 26, cachelines: 1, members: 8 */
	/* last cacheline: 26 bytes */
};

This eliminates the following warning reported by checkpatch:
WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)

---
 drivers/staging/rtl8192e/rtllib_tx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/staging/rtl8192e/rtllib_tx.c +3 -3
index 4f68ffe..de74e0e 100644
@@ -688,12 +688,12 @@ int rtllib_xmit_inter(struct sk_buff *skb, struct net_device *dev)
 				memcpy(&header.addr3,
 				       ieee->current_network.bssid, ETH_ALEN);
 			else
-				memcpy(&header.addr3, &dest, ETH_ALEN);
+				ether_addr_copy(&header.addr3, &dest);
 		} else if (ieee->iw_mode == IW_MODE_ADHOC) {
 			/* not From/To DS: Addr1 = DA, Addr2 = SA,
 			Addr3 = BSSID */
-			memcpy(&header.addr1, dest, ETH_ALEN);
-			memcpy(&header.addr2, src, ETH_ALEN);
+			ether_addr_copy(&header.addr1, dest);
+			ether_addr_copy(&header.addr2, src);
 			memcpy(&header.addr3, ieee->current_network.bssid,
 			       ETH_ALEN);
 		}
Signature · 1 line
1.9.1
#9 lore raw

PATCH 5/5 Staging: rtl8192e: replace memcpy with ether_addr_copy for aligned structures

In reply to Darshana Padmadas

This patch replaces memcpy with ether_addr_copy and eliminates the following warning:
WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)

Pahole is used to show the structures for entry, hdr, rxb struct rtllib_frag_entry {

        long unsigned int          first_frag_time;      /*     0     8 */
        unsigned int               seq;                  /*     8     4 */
        unsigned int               last_frag;            /*    12     4 */
        struct sk_buff *           skb;                  /*    16     8 */
        u8                         src_addr[6];          /*    24     6 */
        u8                         dst_addr[6];          /*    30     6 */
        /* size: 40, cachelines: 1, members: 6 */
        /* padding: 4 */
        /* last cacheline: 40 bytes */
};
struct rtllib_hdr_4addr {
        __le16                     frame_ctl;            /*     0     2 */
        __le16                     duration_id;          /*     2     2 */
        u8                         addr1[6];             /*     4     6 */
        u8                         addr2[6];             /*    10     6 */
        u8                         addr3[6];             /*    16     6 */
        __le16                     seq_ctl;              /*    22     2 */
        u8                         addr4[6];             /*    24     6 */
        u8                         payload[0];           /*    30     0 */
        /* size: 30, cachelines: 1, members: 8 */
        /* last cacheline: 30 bytes */
};
struct rtllib_rxb {
	u8                         nr_subframes;         /*     0     1 */
	struct sk_buff *           subframes[64];        /*     1   512 */
	/* --- cacheline 8 boundary (512 bytes) was 1 bytes ago --- */
	u8                         dst[6];               /*   513     6 */
	u8                         src[6];               /*   519     6 */

	/* size: 525, cachelines: 9, members: 4 */
	/* last cacheline: 13 bytes */
};
---
 drivers/staging/rtl8192e/rtllib_rx.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)
drivers/staging/rtl8192e/rtllib_rx.c +17 -17
index e8261ae..bc13f5d 100644
@@ -139,8 +139,8 @@ rtllib_frag_cache_get(struct rtllib_device *ieee,
 		entry->seq = seq;
 		entry->last_frag = frag;
 		entry->skb = skb;
-		memcpy(entry->src_addr, hdr->addr2, ETH_ALEN);
-		memcpy(entry->dst_addr, hdr->addr1, ETH_ALEN);
+		ether_addr_copy(entry->src_addr, hdr->addr2);
+		ether_addr_copy(entry->dst_addr, hdr->addr1);
 	} else {
 		/* received a fragment of a frame for which the head fragment
 		 * should have already been received */
@@ -397,7 +397,7 @@ static int is_duplicate_packet(struct rtllib_device *ieee,
 			if (!entry)
 				return 0;
 
-			memcpy(entry->mac, mac, ETH_ALEN);
+			ether_addr_copy(entry->mac, mac);
 			entry->seq_num[tid] = seq;
 			entry->frag_num[tid] = frag;
 			entry->packet_time[tid] = jiffies;
@@ -777,14 +777,14 @@ static u8 parse_subframe(struct rtllib_device *ieee, struct sk_buff *skb,
 
 		rxb->subframes[0] = sub_skb;
 
-		memcpy(rxb->src, src, ETH_ALEN);
-		memcpy(rxb->dst, dst, ETH_ALEN);
+		ether_addr_copy(rxb->src, src);
+		ether_addr_copy(rxb->dst, dst);
 		rxb->subframes[0]->dev = ieee->dev;
 		return 1;
 	} else {
 		rxb->nr_subframes = 0;
-		memcpy(rxb->src, src, ETH_ALEN);
-		memcpy(rxb->dst, dst, ETH_ALEN);
+		ether_addr_copy(rxb->src, src);
+		ether_addr_copy(rxb->dst, dst);
 		while (skb->len > ETHERNET_HEADER_SIZE) {
 			/* Offset 12 denote 2 mac address */
 			nSubframe_Length = *((u16 *)(skb->data + 12));
@@ -917,23 +917,23 @@ static void rtllib_rx_extract_addr(struct rtllib_device *ieee,
 
 	switch (fc & (RTLLIB_FCTL_FROMDS | RTLLIB_FCTL_TODS)) {
 	case RTLLIB_FCTL_FROMDS:
-		memcpy(dst, hdr->addr1, ETH_ALEN);
-		memcpy(src, hdr->addr3, ETH_ALEN);
-		memcpy(bssid, hdr->addr2, ETH_ALEN);
+		ether_addr_copy(dst, hdr->addr1);
+		ether_addr_copy(src, hdr->addr3);
+		ether_addr_copy(bssid, hdr->addr2);
 		break;
 	case RTLLIB_FCTL_TODS:
-		memcpy(dst, hdr->addr3, ETH_ALEN);
-		memcpy(src, hdr->addr2, ETH_ALEN);
-		memcpy(bssid, hdr->addr1, ETH_ALEN);
+		ether_addr_copy(dst, hdr->addr3);
+		ether_addr_copy(src, hdr->addr2);
+		ether_addr_copy(bssid, hdr->addr1);
 		break;
 	case RTLLIB_FCTL_FROMDS | RTLLIB_FCTL_TODS:
-		memcpy(dst, hdr->addr3, ETH_ALEN);
-		memcpy(src, hdr->addr4, ETH_ALEN);
+		ether_addr_copy(dst, hdr->addr3);
+		ether_addr_copy(src, hdr->addr4);
 		memcpy(bssid, ieee->current_network.bssid, ETH_ALEN);
 		break;
 	case 0:
-		memcpy(dst, hdr->addr1, ETH_ALEN);
-		memcpy(src, hdr->addr2, ETH_ALEN);
+		ether_addr_copy(dst, hdr->addr1);
+		ether_addr_copy(src, hdr->addr2);
 		memcpy(bssid, hdr->addr3, ETH_ALEN);
 		break;
 	}
Signature · 1 line
1.9.1
#10 lore raw
In reply to Darshana Padmadas
Quoted 15 lines · Darshana Padmadas

This patch set replaces memcpy with ether_addr_copy if the addresses of the structures are aligned. Structure layout is shown by using the Pahole tool.

This patch set fixes the following warning reported by checkpatch:
WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)

Darshana Padmadas (5):
Staging: rtl8192e: Replace memcpy with ether_addr_copy
Staging: rtl8192e: Use ether_addr_copy instead of memcpy
Staging: rtl8192e: Use ether_addr_copy replacing memcpy
Staging: rtl8192e: Use ether_addr_copy for aligned addresses
Staging: rtl8192e: replace memcpy with ether_addr_copy for aligned
structures

Quoted 7 lines
 drivers/staging/rtl8192e/rtl819x_BAProc.c    |  4 ++--
 drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 10 ++++----
 drivers/staging/rtl8192e/rtllib_rx.c         | 34 ++++++++++++++--------------
 drivers/staging/rtl8192e/rtllib_softmac.c    | 12 +++++-----
 drivers/staging/rtl8192e/rtllib_tx.c         |  6 ++---
 5 files changed, 33 insertions(+), 33 deletions(-)

This set looks good to me!

Jes

maintainer
#11 lore raw
In reply to Darshana Padmadas
Quoted 15 lines · Darshana Padmadas

This patch set replaces memcpy with ether_addr_copy if the addresses of the structures are aligned. Structure layout is shown by using the Pahole tool.

This patch set fixes the following warning reported by checkpatch:
WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)

Darshana Padmadas (5):
Staging: rtl8192e: Replace memcpy with ether_addr_copy
Staging: rtl8192e: Use ether_addr_copy instead of memcpy
Staging: rtl8192e: Use ether_addr_copy replacing memcpy
Staging: rtl8192e: Use ether_addr_copy for aligned addresses
Staging: rtl8192e: replace memcpy with ether_addr_copy for aligned
structures

Quoted 6 lines
 drivers/staging/rtl8192e/rtl819x_BAProc.c    |  4 ++--
 drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 10 ++++----
 drivers/staging/rtl8192e/rtllib_rx.c         | 34 ++++++++++++++--------------
 drivers/staging/rtl8192e/rtllib_softmac.c    | 12 +++++-----
 drivers/staging/rtl8192e/rtllib_tx.c         |  6 ++---
 5 files changed, 33 insertions(+), 33 deletions(-)

You did not test build these patches, they break the build horribly :(

ALWAYS test-build your patches before sending them out, it's rude to have to have other people find problems with them.

thanks,

greg k-h

#2 lore raw

PATCH v2 1/5 Staging: rtl8192e: Replace memcpy with ether_addr_copy

In reply to Darshana Padmadas

This patch fixes the following warning found by checkpatch.pl:
Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2

Also included the header file linux/etherdevice.h where ether_addr_copy has been defined.
Pahole showed that the structure for BAReq and Delba is aligned:
struct rtllib_hdr_3addr {

        __le16                     frame_ctl;            /*     0     2 */
        __le16                     duration_id;          /*     2     2 */
        u8                         addr1[6];             /*     4     6 */
        u8                         addr2[6];             /*    10     6 */
        u8                         addr3[6];             /*    16     6 */
        __le16                     seq_ctl;              /*    22     2 */
        u8                         payload[0];           /*    24     0 */
        /* size: 24, cachelines: 1, members: 7 */
        /* last cacheline: 24 bytes */
};

---
Changes in v2:
- Included header file where ether_addr_copy is defined.
- Made the commit message more clear.

 drivers/staging/rtl8192e/rtl819x_BAProc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/staging/rtl8192e/rtl819x_BAProc.c +3 -2
index 0415e02..4031462 100644
@@ -18,6 +18,7 @@
 ******************************************************************************/
 #include <asm/byteorder.h>
 #include <asm/unaligned.h>
+#include <linux/etherdevice.h>
 #include "rtllib.h"
 #include "rtl819x_BA.h"
 
@@ -103,7 +104,7 @@ static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst,
 	BAReq = (struct rtllib_hdr_3addr *)skb_put(skb,
 		 sizeof(struct rtllib_hdr_3addr));
 
-	memcpy(BAReq->addr1, Dst, ETH_ALEN);
+	ether_addr_copy(BAReq->addr1, Dst);
 	memcpy(BAReq->addr2, ieee->dev->dev_addr, ETH_ALEN);
 
 	memcpy(BAReq->addr3, ieee->current_network.bssid, ETH_ALEN);
@@ -168,7 +169,7 @@ static struct sk_buff *rtllib_DELBA(struct rtllib_device *ieee, u8 *dst,
 	Delba = (struct rtllib_hdr_3addr *) skb_put(skb,
 		 sizeof(struct rtllib_hdr_3addr));
 
-	memcpy(Delba->addr1, dst, ETH_ALEN);
+	ether_addr_copy(Delba->addr1, dst);
 	memcpy(Delba->addr2, ieee->dev->dev_addr, ETH_ALEN);
 	memcpy(Delba->addr3, ieee->current_network.bssid, ETH_ALEN);
 	Delba->frame_ctl = cpu_to_le16(RTLLIB_STYPE_MANAGE_ACT);
Signature · 1 line
1.9.1
#5 lore raw

PATCH v2 2/5 Staging: rtl8192e: Use ether_addr_copy instead of memcpy

In reply to Darshana Padmadas

This patch replaces use of memcpy with ether_addr_copy since the addresses for struct of hdr11 are aligned as shown by Pahole and hdr is of type u8.
The header file linux/etherdevice.h is also included where ether_addr_copy is defined.

struct rtllib_hdr_4addr {

        __le16                     frame_ctl;            /*     0     2 */
        __le16                     duration_id;          /*     2     2 */
        u8                         addr1[6];             /*     4     6 */
        u8                         addr2[6];             /*    10     6 */
        u8                         addr3[6];             /*    16     6 */
        __le16                     seq_ctl;              /*    22     2 */
        u8                         addr4[6];             /*    24     6 */
        u8                         payload[0];           /*    30     0 */
        /* size: 30, cachelines: 1, members: 8 */
        /* last cacheline: 30 bytes */
};

---
Changes in v2:
- Included header file where ether_addr_copy is defined.
- Made commit message more clear.

 drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/staging/rtl8192e/rtllib_crypt_tkip.c +6 -6
index 78db2b6..949fc8f 100644
@@ -21,7 +21,7 @@
 #include <linux/crypto.h>
 #include <linux/scatterlist.h>
 #include <linux/crc32.h>
-
+#include <linux/etherdevice.h>
 #include "rtllib.h"
 
 struct rtllib_tkip_data {
@@ -525,19 +525,19 @@ static void michael_mic_hdr(struct sk_buff *skb, u8 *hdr)
 	switch (le16_to_cpu(hdr11->frame_ctl) &
 		(RTLLIB_FCTL_FROMDS | RTLLIB_FCTL_TODS)) {
 	case RTLLIB_FCTL_TODS:
-		memcpy(hdr, hdr11->addr3, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr3); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN); /* SA */
 		break;
 	case RTLLIB_FCTL_FROMDS:
-		memcpy(hdr, hdr11->addr1, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr1); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr3, ETH_ALEN); /* SA */
 		break;
 	case RTLLIB_FCTL_FROMDS | RTLLIB_FCTL_TODS:
-		memcpy(hdr, hdr11->addr3, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr3); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr4, ETH_ALEN); /* SA */
 		break;
 	case 0:
-		memcpy(hdr, hdr11->addr1, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr1); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN); /* SA */
 		break;
 	}
@@ -591,7 +591,7 @@ static void rtllib_michael_mic_failure(struct net_device *dev,
 	else
 		ev.flags |= IW_MICFAILURE_PAIRWISE;
 	ev.src_addr.sa_family = ARPHRD_ETHER;
-	memcpy(ev.src_addr.sa_data, hdr->addr2, ETH_ALEN);
+	ether_addr_copy(ev.src_addr.sa_data, hdr->addr2);
 	memset(&wrqu, 0, sizeof(wrqu));
 	wrqu.data.length = sizeof(ev);
 	wireless_send_event(dev, IWEVMICHAELMICFAILURE, &wrqu, (char *) &ev);
Signature · 1 line
1.9.1
#3 lore raw

Re: [Outreachy kernel] [PATCH v2 1/5] Staging: rtl8192e: Replace memcpy with ether_addr_copy

In reply to Darshana Padmadas

I think that both addresses have to be aligned?

julia

Quoted 68 lines · Darshana Padmadas

This patch fixes the following warning found by checkpatch.pl:
Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2

Also included the header file linux/etherdevice.h where ether_addr_copy has been defined.
Pahole showed that the structure for BAReq and Delba is aligned:
struct rtllib_hdr_3addr {

        __le16                     frame_ctl;            /*     0     2 */
        __le16                     duration_id;          /*     2     2 */
        u8                         addr1[6];             /*     4     6 */
        u8                         addr2[6];             /*    10     6 */
        u8                         addr3[6];             /*    16     6 */
        __le16                     seq_ctl;              /*    22     2 */
        u8                         payload[0];           /*    24     0 */
        /* size: 24, cachelines: 1, members: 7 */
        /* last cacheline: 24 bytes */
};

---
Changes in v2:
- Included header file where ether_addr_copy is defined.
- Made the commit message more clear.

 drivers/staging/rtl8192e/rtl819x_BAProc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/staging/rtl8192e/rtl819x_BAProc.c +3 -3
index 0415e02..4031462 100644
@@ -18,6 +18,7 @@
 ******************************************************************************/
 #include <asm/byteorder.h>
 #include <asm/unaligned.h>
+#include <linux/etherdevice.h>
 #include "rtllib.h"
 #include "rtl819x_BA.h"
 
@@ -103,7 +104,7 @@ static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst,
 	BAReq = (struct rtllib_hdr_3addr *)skb_put(skb,
 		 sizeof(struct rtllib_hdr_3addr));
 
-	memcpy(BAReq->addr1, Dst, ETH_ALEN);
+	ether_addr_copy(BAReq->addr1, Dst);
 	memcpy(BAReq->addr2, ieee->dev->dev_addr, ETH_ALEN);
 
 	memcpy(BAReq->addr3, ieee->current_network.bssid, ETH_ALEN);
@@ -168,7 +169,7 @@ static struct sk_buff *rtllib_DELBA(struct rtllib_device *ieee, u8 *dst,
 	Delba = (struct rtllib_hdr_3addr *) skb_put(skb,
 		 sizeof(struct rtllib_hdr_3addr));
 
-	memcpy(Delba->addr1, dst, ETH_ALEN);
+	ether_addr_copy(Delba->addr1, dst);
 	memcpy(Delba->addr2, ieee->dev->dev_addr, ETH_ALEN);
 	memcpy(Delba->addr3, ieee->current_network.bssid, ETH_ALEN);
 	Delba->frame_ctl = cpu_to_le16(RTLLIB_STYPE_MANAGE_ACT);
--

1.9.1

--
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 [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/9eab4a1c89f930dac0710fda702a6de414172237.1426681038.git.darshanapadmadas%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

#6 lore raw

PATCH v3 2/5 Staging: rtl8192e: Use ether_addr_copy instead of memcpy

In reply to Darshana Padmadas

This patch replaces use of memcpy with ether_addr_copy since the addresses for struct of hdr11 and ev are aligned as shown by Pahole and hdr is of type u8.
The header file linux/etherdevice.h is also included where ether_addr_copy is defined.

struct rtllib_hdr_4addr {

        __le16                     frame_ctl;            /*     0     2 */
        __le16                     duration_id;          /*     2     2 */
        u8                         addr1[6];             /*     4     6 */
        u8                         addr2[6];             /*    10     6 */
        u8                         addr3[6];             /*    16     6 */
        __le16                     seq_ctl;              /*    22     2 */
        u8                         addr4[6];             /*    24     6 */
        u8                         payload[0];           /*    30     0 */
        /* size: 30, cachelines: 1, members: 8 */
        /* last cacheline: 30 bytes */
};
struct iw_michaelmicfailure {
	__u32                      flags;                /*     0     4 */
	struct sockaddr            src_addr;             /*     4    16 */
	__u8                       tsc[8];               /*    20     8 */
	/* size: 28, cachelines: 1, members: 3 */
	/* last cacheline: 28 bytes */
};

---
Changes in v3:
- Included structure layout of ev in commit message.

 drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/staging/rtl8192e/rtllib_crypt_tkip.c +6 -6
index 78db2b6..949fc8f 100644
@@ -21,7 +21,7 @@
 #include <linux/crypto.h>
 #include <linux/scatterlist.h>
 #include <linux/crc32.h>
-
+#include <linux/etherdevice.h>
 #include "rtllib.h"
 
 struct rtllib_tkip_data {
@@ -525,19 +525,19 @@ static void michael_mic_hdr(struct sk_buff *skb, u8 *hdr)
 	switch (le16_to_cpu(hdr11->frame_ctl) &
 		(RTLLIB_FCTL_FROMDS | RTLLIB_FCTL_TODS)) {
 	case RTLLIB_FCTL_TODS:
-		memcpy(hdr, hdr11->addr3, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr3); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN); /* SA */
 		break;
 	case RTLLIB_FCTL_FROMDS:
-		memcpy(hdr, hdr11->addr1, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr1); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr3, ETH_ALEN); /* SA */
 		break;
 	case RTLLIB_FCTL_FROMDS | RTLLIB_FCTL_TODS:
-		memcpy(hdr, hdr11->addr3, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr3); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr4, ETH_ALEN); /* SA */
 		break;
 	case 0:
-		memcpy(hdr, hdr11->addr1, ETH_ALEN); /* DA */
+		ether_addr_copy(hdr, hdr11->addr1); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN); /* SA */
 		break;
 	}
@@ -591,7 +591,7 @@ static void rtllib_michael_mic_failure(struct net_device *dev,
 	else
 		ev.flags |= IW_MICFAILURE_PAIRWISE;
 	ev.src_addr.sa_family = ARPHRD_ETHER;
-	memcpy(ev.src_addr.sa_data, hdr->addr2, ETH_ALEN);
+	ether_addr_copy(ev.src_addr.sa_data, hdr->addr2);
 	memset(&wrqu, 0, sizeof(wrqu));
 	wrqu.data.length = sizeof(ev);
 	wireless_send_event(dev, IWEVMICHAELMICFAILURE, &wrqu, (char *) &ev);
Signature · 1 line
1.9.1
#12 lore raw
In reply to Greg KH

Sorry, about that. Have sent v2 for patches 1/5 and 2/5 that broke the build.

Quoted 35 lines · gregkh
Quoted 6 lines · Darshana Padmadas

This patch set replaces memcpy with ether_addr_copy if the addresses of the structures are aligned. Structure layout is shown by using the Pahole tool.

This patch set fixes the following warning reported by checkpatch:
WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet

addresses are __aligned(2)

Quoted 9 lines

Darshana Padmadas (5):
Staging: rtl8192e: Replace memcpy with ether_addr_copy
Staging: rtl8192e: Use ether_addr_copy instead of memcpy
Staging: rtl8192e: Use ether_addr_copy replacing memcpy
Staging: rtl8192e: Use ether_addr_copy for aligned addresses
Staging: rtl8192e: replace memcpy with ether_addr_copy for aligned
structures

Quoted 3 lines
 drivers/staging/rtl8192e/rtl819x_BAProc.c    |  4 ++-- 
 drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 10 ++++---- 
 drivers/staging/rtl8192e/rtllib_rx.c         | 34 

++++++++++++++--------------

Quoted 3 lines
 drivers/staging/rtl8192e/rtllib_softmac.c    | 12 +++++----- 
 drivers/staging/rtl8192e/rtllib_tx.c         |  6 ++--- 
 5 files changed, 33 insertions(+), 33 deletions(-) 

You did not test build these patches, they break the build horribly :(

ALWAYS test-build your patches before sending them out, it's rude to have to have other people find problems with them.

thanks,

greg k-h

maintainer
#13 lore raw
Quoted 1 line · Darshana Padmadas

Sorry, about that. Have sent v2 for patches 1/5 and 2/5 that broke the build.

Please resend everything, the whole patch set is long-gone from my queue.

thanks,

greg k-h