2015-05-17 21:16:45

by Jagan Teki

Subject: [PATCH] staging: rtl8712: Use ether_addr_copy() instead of memcpy()

This patch fixes to use ether_addr_copy() instead of memcpy()

Encounter this by applying checkpatch.pl against this file:
WARNING: Prefer ether_addr_copy() over memcpy()
if the Ethernet addresses are __aligned(2)

Signed-off-by: Jagan Teki <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Larry Finger <[email protected]>
Cc: Florian Schilhabel <[email protected]>
---
drivers/staging/rtl8712/os_intfs.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
index 6e776e5..0d27df6 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -181,7 +181,7 @@ static int r871x_net_set_mac_address(struct net_device *pnetdev, void *p)
struct sockaddr *addr = p;

if (padapter->bup == false)
- memcpy(pnetdev->dev_addr, addr->sa_data, ETH_ALEN);
+ ether_addr_copy(pnetdev->dev_addr, addr->sa_data);
return 0;
}

@@ -395,8 +395,8 @@ static int netdev_open(struct net_device *pnetdev)
goto netdev_open_error;
if (r8712_initmac == NULL)
/* Use the mac address stored in the Efuse */
- memcpy(pnetdev->dev_addr,
- padapter->eeprompriv.mac_addr, ETH_ALEN);
+ ether_addr_copy(pnetdev->dev_addr,
+ padapter->eeprompriv.mac_addr);
else {
/* We have to inform f/w to use user-supplied MAC
* address.
@@ -412,8 +412,8 @@ static int netdev_open(struct net_device *pnetdev)
* the eeprompriv.mac_addr should store the mac which
* users specify.
*/
- memcpy(padapter->eeprompriv.mac_addr,
- pnetdev->dev_addr, ETH_ALEN);
+ ether_addr_copy(padapter->eeprompriv.mac_addr,
+ pnetdev->dev_addr);
}
if (start_drv_threads(padapter) != _SUCCESS)
goto netdev_open_error;
--
1.9.1

2015-05-17 23:27:20

by Greg Kroah-Hartman

Subject: Re: [PATCH] staging: rtl8712: Use ether_addr_copy() instead of memcpy()

On Mon, May 18, 2015 at 02:27:01AM +0530, Jagan Teki wrote:
> This patch fixes to use ether_addr_copy() instead of memcpy()
>
> Encounter this by applying checkpatch.pl against this file:
> WARNING: Prefer ether_addr_copy() over memcpy()
> if the Ethernet addresses are __aligned(2)
>
> Signed-off-by: Jagan Teki <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Larry Finger <[email protected]>
> Cc: Florian Schilhabel <[email protected]>
> ---
> drivers/staging/rtl8712/os_intfs.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)

You need to prove that the addresses are aligned, please do so in the
changelog entry.

2015-05-18 00:09:57

by Larry Finger

Subject: Re: [PATCH] staging: rtl8712: Use ether_addr_copy() instead of memcpy()

On 05/17/2015 03:57 PM, Jagan Teki wrote:
> This patch fixes to use ether_addr_copy() instead of memcpy()
>
> Encounter this by applying checkpatch.pl against this file:
> WARNING: Prefer ether_addr_copy() over memcpy()
> if the Ethernet addresses are __aligned(2)
>
> Signed-off-by: Jagan Teki <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Larry Finger <[email protected]>
> Cc: Florian Schilhabel <[email protected]>
> ---

Can you assure us that you have carefully checked the the source address of
addr->sa_data and padapter->eeprompriv.mac_addr are suitably aligned? At a
minimum, have you tested this change on real hardware?

Larry


> drivers/staging/rtl8712/os_intfs.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
> index 6e776e5..0d27df6 100644
> --- a/drivers/staging/rtl8712/os_intfs.c
> +++ b/drivers/staging/rtl8712/os_intfs.c
> @@ -181,7 +181,7 @@ static int r871x_net_set_mac_address(struct net_device *pnetdev, void *p)
> struct sockaddr *addr = p;
>
> if (padapter->bup == false)
> - memcpy(pnetdev->dev_addr, addr->sa_data, ETH_ALEN);
> + ether_addr_copy(pnetdev->dev_addr, addr->sa_data);
> return 0;
> }
>
> @@ -395,8 +395,8 @@ static int netdev_open(struct net_device *pnetdev)
> goto netdev_open_error;
> if (r8712_initmac == NULL)
> /* Use the mac address stored in the Efuse */
> - memcpy(pnetdev->dev_addr,
> - padapter->eeprompriv.mac_addr, ETH_ALEN);
> + ether_addr_copy(pnetdev->dev_addr,
> + padapter->eeprompriv.mac_addr);
> else {
> /* We have to inform f/w to use user-supplied MAC
> * address.
> @@ -412,8 +412,8 @@ static int netdev_open(struct net_device *pnetdev)
> * the eeprompriv.mac_addr should store the mac which
> * users specify.
> */
> - memcpy(padapter->eeprompriv.mac_addr,
> - pnetdev->dev_addr, ETH_ALEN);
> + ether_addr_copy(padapter->eeprompriv.mac_addr,
> + pnetdev->dev_addr);
> }
> if (start_drv_threads(padapter) != _SUCCESS)
> goto netdev_open_error;
>