Replace memcpy with ether_addr_copy and fix checkpatch warning
v2 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):
Changes in v2:
- Included header files that define ether_addr_copy in
patch 1/5 and 2/5.
- Edited the commit message more clear in 1/5 and 2/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 | 5 +++-- drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 12 +++++------ drivers/staging/rtl8192e/rtllib_rx.c | 32 ++++++++++++++-------------- drivers/staging/rtl8192e/rtllib_softmac.c | 12 +++++------ drivers/staging/rtl8192e/rtllib_tx.c | 6 +++--- 5 files changed, 34 insertions(+), 33 deletions(-)
Signature · 1 line
1.9.1
PATCH v2 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
Also included header file linux/etherdevice.h that defines ether_addr_copy.
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 */
};Signed-off-by Darshana Padmadas <[email protected]>
--- drivers/staging/rtl8192e/rtl819x_BAProc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/staging/rtl8192e/rtl819x_BAProc.c +3 -2
@@ -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
PATCH v2 2/5 Staging: rtl8192e: Use ether_addr_copy instead of memcpy
In reply to Darshana PadmadasThis 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 */ };
Signed-off-by Darshana Padmadas <[email protected]>
--- drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/staging/rtl8192e/rtllib_crypt_tkip.c +6 -6
@@ -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
PATCH v2 3/5 Staging: rtl8192e: Use ether_addr_copy replacing memcpy
In reply to Darshana PadmadasThis 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 */
};Signed-off-by Darshana Padmadas <[email protected]>
--- drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/staging/rtl8192e/rtllib_softmac.c +6 -6
@@ -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
PATCH v2 4/5 Staging: rtl8192e: Use ether_addr_copy for aligned addresses
In reply to Darshana PadmadasThis 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 */
};Signed-off-by Darshana Padmadas <[email protected]>
--- drivers/staging/rtl8192e/rtllib_tx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/staging/rtl8192e/rtllib_tx.c +3 -3
@@ -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
PATCH v2 5/5 Staging: rtl8192e: replace memcpy with ether_addr_copy for aligned structures
In reply to Darshana PadmadasThis 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 */
};Signed-off-by Darshana Padmadas <[email protected]>
--- drivers/staging/rtl8192e/rtllib_rx.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
drivers/staging/rtl8192e/rtllib_rx.c +16 -16
@@ -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;
@@ -783,8 +783,8 @@ static u8 parse_subframe(struct rtllib_device *ieee, struct sk_buff *skb,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,24 +917,24 @@ 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 ); -memcpy (bssid, hdr->addr3,ETH_ALEN ); +ether_addr_copy (dst, hdr->addr1); +ether_addr_copy (src, hdr->addr2); +ether_addr_copy (bssid, hdr->addr3);break ; } }
Signature · 1 line
1.9.1
Re: [PATCH v2 4/5] Staging: rtl8192e: Use ether_addr_copy for aligned addresses
In reply to Darshana PadmadasThis patch brings up incompatible pointer type warning for some arguments &dest and
&header. So, please ignore this patch.
Also should I send v3 for this patch set?
Quoted 50 lines · Darshana Padmadas
This patch replaces memcpy with
ether_addr_copysince structures are aligned as shown by pahole:
structrtllib_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 */ };Signed-off-by Darshana Padmadas <[email protected]>---
drivers/staging/rtl8192e/rtllib_tx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/staging/rtl8192e/rtllib_tx.c +3
-3@@ -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
Greg KH
maintainerRe: [Outreachy kernel] [PATCH v2 1/5] Staging: rtl8192e: Replace memcpy with ether_addr_copy
In reply to Darshana PadmadasQuoted 5 lines · Darshana Padmadas
This patch fixes the following warning found by checkpatch.pl:
Preferether_addr_copy()overmemcpy()if the Ethernet addresses are __aligned(2Also included header file
linux/etherdevice.hthat definesether_addr_copy.
<snip>
This series still adds kernel build warnings, which is not allowed. It looks like the patches are actually incorrect, and the compiler is correctly warning you about this.
Please fix this up, TEST THEM!!!, and then resend.
thanks,
greg k-h