All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: rtl8188eu: hal: rtl8188e_cmd: Use ether_addr_copy() instead of memcpy()
@ 2016-09-20 12:57 Georgiana Rodica Chelu
  2016-09-22  7:14 ` Greg KH
  0 siblings, 1 reply; 3+ messages in thread
From: Georgiana Rodica Chelu @ 2016-09-20 12:57 UTC (permalink / raw)
  To: outreachy-kernel; +Cc: gregkh

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

Checked if the the Ethernet addresses are __aligned(2) by using pahole
tool. The type of pwlanhdr is struct ieee80211_hdr and pahole shows that
addr1, addr2, and addr3 are aligned to u16.

struct ieee80211_hdr {
	__le16                     frame_control;        /*     0     2
*/
	__le16                     duration_id;          /*     2     2
*/
	u8                         addr1[6];             /*     4     6
*/
	u8                         addr2[6];             /*    10     6
*/
	u8                         addr3[6];             /*    16     6
*/
	__le16                     seq_ctrl;             /*    22     2
*/
	u8                         addr4[6];             /*    24     6
*/

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

Both eeprompriv from struct adapter and MacAddress from struct
wlan_bssid_ex have the offset multiple of sizeof(u16).

Also, the array bc_addr and the pointers: StaAddr, mac, and bssid,
start from an even offset.

Signed-off-by: Georgiana Rodica Chelu <georgiana.chelu93@gmail.com>
---
 drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c | 34 ++++++++++++++--------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
index 18f69b8..5bd47c8 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
@@ -232,9 +232,9 @@ static void ConstructBeacon(struct adapter *adapt, u8 *pframe, u32 *pLength)
 	fctrl = &pwlanhdr->frame_control;
 	*(fctrl) = 0;
 
-	memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
-	memcpy(pwlanhdr->addr2, myid(&(adapt->eeprompriv)), ETH_ALEN);
-	memcpy(pwlanhdr->addr3, cur_network->MacAddress, ETH_ALEN);
+	ether_addr_copy(pwlanhdr->addr1, bc_addr);
+	ether_addr_copy(pwlanhdr->addr2, myid(&(adapt->eeprompriv)));
+	ether_addr_copy(pwlanhdr->addr3, cur_network->MacAddress);
 
 	SetSeqNum(pwlanhdr, 0/*pmlmeext->mgnt_seq*/);
 	SetFrameSubType(pframe, WIFI_BEACON);
@@ -322,10 +322,10 @@ static void ConstructPSPoll(struct adapter *adapt, u8 *pframe, u32 *pLength)
 	SetDuration(pframe, (pmlmeinfo->aid | 0xc000));
 
 	/*  BSSID. */
-	memcpy(pwlanhdr->addr1, pnetwork->MacAddress, ETH_ALEN);
+	ether_addr_copy(pwlanhdr->addr1, pnetwork->MacAddress);
 
 	/*  TA. */
-	memcpy(pwlanhdr->addr2, myid(&(adapt->eeprompriv)), ETH_ALEN);
+	ether_addr_copy(pwlanhdr->addr2, myid(&(adapt->eeprompriv)));
 
 	*pLength = 16;
 }
@@ -357,21 +357,21 @@ static void ConstructNullFunctionData(struct adapter *adapt, u8 *pframe,
 	switch (cur_network->network.InfrastructureMode) {
 	case Ndis802_11Infrastructure:
 		SetToDs(fctrl);
-		memcpy(pwlanhdr->addr1, pnetwork->MacAddress, ETH_ALEN);
-		memcpy(pwlanhdr->addr2, myid(&(adapt->eeprompriv)), ETH_ALEN);
-		memcpy(pwlanhdr->addr3, StaAddr, ETH_ALEN);
+		ether_addr_copy(pwlanhdr->addr1, pnetwork->MacAddress);
+		ether_addr_copy(pwlanhdr->addr2, myid(&(adapt->eeprompriv)));
+		ether_addr_copy(pwlanhdr->addr3, StaAddr);
 		break;
 	case Ndis802_11APMode:
 		SetFrDs(fctrl);
-		memcpy(pwlanhdr->addr1, StaAddr, ETH_ALEN);
-		memcpy(pwlanhdr->addr2, pnetwork->MacAddress, ETH_ALEN);
-		memcpy(pwlanhdr->addr3, myid(&(adapt->eeprompriv)), ETH_ALEN);
+		ether_addr_copy(pwlanhdr->addr1, StaAddr);
+		ether_addr_copy(pwlanhdr->addr2, pnetwork->MacAddress);
+		ether_addr_copy(pwlanhdr->addr3, myid(&(adapt->eeprompriv)));
 		break;
 	case Ndis802_11IBSS:
 	default:
-		memcpy(pwlanhdr->addr1, StaAddr, ETH_ALEN);
-		memcpy(pwlanhdr->addr2, myid(&(adapt->eeprompriv)), ETH_ALEN);
-		memcpy(pwlanhdr->addr3, pnetwork->MacAddress, ETH_ALEN);
+		ether_addr_copy(pwlanhdr->addr1, StaAddr);
+		ether_addr_copy(pwlanhdr->addr2, myid(&(adapt->eeprompriv)));
+		ether_addr_copy(pwlanhdr->addr3, pnetwork->MacAddress);
 		break;
 	}
 
@@ -413,9 +413,9 @@ static void ConstructProbeRsp(struct adapter *adapt, u8 *pframe, u32 *pLength, u
 
 	fctrl = &pwlanhdr->frame_control;
 	*(fctrl) = 0;
-	memcpy(pwlanhdr->addr1, StaAddr, ETH_ALEN);
-	memcpy(pwlanhdr->addr2, mac, ETH_ALEN);
-	memcpy(pwlanhdr->addr3, bssid, ETH_ALEN);
+	ether_addr_copy(pwlanhdr->addr1, StaAddr);
+	ether_addr_copy(pwlanhdr->addr2, mac);
+	ether_addr_copy(pwlanhdr->addr3, bssid);
 
 	SetSeqNum(pwlanhdr, 0);
 	SetFrameSubType(fctrl, WIFI_PROBERSP);
-- 
2.7.4



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

* Re: [PATCH] staging: rtl8188eu: hal: rtl8188e_cmd: Use ether_addr_copy() instead of memcpy()
  2016-09-20 12:57 [PATCH] staging: rtl8188eu: hal: rtl8188e_cmd: Use ether_addr_copy() instead of memcpy() Georgiana Rodica Chelu
@ 2016-09-22  7:14 ` Greg KH
  2016-09-23  6:16   ` Georgiana Chelu
  0 siblings, 1 reply; 3+ messages in thread
From: Greg KH @ 2016-09-22  7:14 UTC (permalink / raw)
  To: Georgiana Rodica Chelu; +Cc: outreachy-kernel

On Tue, Sep 20, 2016 at 03:57:40PM +0300, Georgiana Rodica Chelu wrote:
> The checkpatch.pl found the warning:
> WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet
> addresses are __aligned(2)
> 
> Checked if the the Ethernet addresses are __aligned(2) by using pahole
> tool. The type of pwlanhdr is struct ieee80211_hdr and pahole shows that
> addr1, addr2, and addr3 are aligned to u16.
> 
> struct ieee80211_hdr {
> 	__le16                     frame_control;        /*     0     2
> */

Your changelog here is line-wrapped, can you fix it up please?

thanks,

greg k-h


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

* Re: [PATCH] staging: rtl8188eu: hal: rtl8188e_cmd: Use ether_addr_copy() instead of memcpy()
  2016-09-22  7:14 ` Greg KH
@ 2016-09-23  6:16   ` Georgiana Chelu
  0 siblings, 0 replies; 3+ messages in thread
From: Georgiana Chelu @ 2016-09-23  6:16 UTC (permalink / raw)
  To: Greg KH; +Cc: outreachy-kernel

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

Thank you for the review. I submitted a second version of the patch.

Georgiana

On 22 September 2016 at 10:14, Greg KH <gregkh@linuxfoundation.org> wrote:

> On Tue, Sep 20, 2016 at 03:57:40PM +0300, Georgiana Rodica Chelu wrote:
> > The checkpatch.pl found the warning:
> > WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet
> > addresses are __aligned(2)
> >
> > Checked if the the Ethernet addresses are __aligned(2) by using pahole
> > tool. The type of pwlanhdr is struct ieee80211_hdr and pahole shows that
> > addr1, addr2, and addr3 are aligned to u16.
> >
> > struct ieee80211_hdr {
> >       __le16                     frame_control;        /*     0     2
> > */
>
> Your changelog here is line-wrapped, can you fix it up please?
>
> thanks,
>
> greg k-h
>

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

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

end of thread, other threads:[~2016-09-23  6:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-20 12:57 [PATCH] staging: rtl8188eu: hal: rtl8188e_cmd: Use ether_addr_copy() instead of memcpy() Georgiana Rodica Chelu
2016-09-22  7:14 ` Greg KH
2016-09-23  6:16   ` Georgiana Chelu

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.