staging: rtl8712: Use ether_addr_copy() instead of memcpy()
PATCH v2 1/1 staging: rtl8712: Use ether_addr_copy() instead of memcpy()
Fixes Warning encounter this by applying checkpatch.pl against this file:
Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)
pahole output for respective structures:
- addr->sa_data
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 */
};- padapter->eeprompriv.mac_addr
struct _adapter {
struct dvobj_priv dvobjpriv; /* 0 40 */
struct mlme_priv mlmepriv; /* 40 1560 */
/* --- cacheline 25 boundary (1600 bytes) --- */
struct cmd_priv cmdpriv; /* 1600 136 */
/* --- cacheline 27 boundary (1728 bytes) was 8 bytes ago --- */
struct evt_priv evtpriv; /* 1736 96 */
/* --- cacheline 28 boundary (1792 bytes) was 40 bytes ago --- */
struct io_queue * pio_queue; /* 1832 8 */
struct xmit_priv xmitpriv; /* 1840 912 */
/* --- cacheline 43 boundary (2752 bytes) --- */
struct recv_priv recvpriv; /* 2752 1088 */
/* --- cacheline 60 boundary (3840 bytes) --- */
struct sta_priv stapriv; /* 3840 672 */
/* --- cacheline 70 boundary (4480 bytes) was 32 bytes ago --- */
struct security_priv securitypriv; /* 4512 4816 */
/* --- cacheline 145 boundary (9280 bytes) was 48 bytes ago --- */
struct registry_priv registrypriv; /* 9328 968 */
/* --- cacheline 160 boundary (10240 bytes) was 56 bytes ago --- */
struct wlan_acl_pool acl_list; /* 10296 1536 */
/* --- cacheline 184 boundary (11776 bytes) was 56 bytes ago --- */
struct pwrctrl_priv pwrctrlpriv; /* 11832 224 */
/* --- cacheline 188 boundary (12032 bytes) was 24 bytes ago --- */
struct eeprom_priv eeprompriv; /* 12056 508 */
........
};struct eeprom_priv {
u8 bautoload_fail_flag; /* 0 1 */
u8 bempty; /* 1 1 */
u8 sys_config; /* 2 1 */
u8 mac_addr[6]; /* 3 6 */
......
};- pnetdev->dev_addrdev_addr is interface address infor from generic net_device structure which is properly aligned and have some patches with this change as well. "staging: rtl8712: fix Prefer ether_addr_copy() over memcpy()" (sha1: 36e4d8826b317080e283e4edd08bf8d5ac706f38)
---
Changes for v2:
- Describe a changelog, to prove address are aligned.
drivers/staging/rtl8712/os_intfs.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/staging/rtl8712/os_intfs.c +5 -5
@@ -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;
Signature · 1 line
1.9.1
Quoted 7 lines · Jagan Teki
struct
eeprom_priv{u8 bautoload_fail_flag; /* 0 1 */ u8 bempty; /* 1 1 */ u8 sys_config; /* 2 1 */ u8 mac_addr[6]; /* 3 6 */ ...... };
This means it's not aligned as we want.
regards,
dan carpenter
Quoted 10 lines · Dan Carpenter
Quoted 7 lines · Jagan Teki
struct
eeprom_priv{u8 bautoload_fail_flag; /* 0 1 */ u8 bempty; /* 1 1 */ u8 sys_config; /* 2 1 */ u8 mac_addr[6]; /* 3 6 */ ...... };This means it's not aligned as we want.
Why size upto mac_addr is 9 and is not divisible by 2 is it?
or the mac_addr size is 6 which is divisible by 2 (__align(2))
this is full pahole:
struct eeprom_priv {
u8 bautoload_fail_flag; /* 0 1 */ u8 bempty; /* 1 1 */ u8 sys_config; /* 2 1 */ u8 mac_addr[6]; /* 3 6 */ u8 config0; /* 9 1 */ u16 channel_plan; /* 10 2 */ u8 country_string[3]; /* 12 3 */ u8 tx_power_b[15]; /* 15 15 */ u8 tx_power_g[15]; /* 30 15 */ u8 tx_power_a[201]; /* 45 201 */
/* --- cacheline 3 boundary (192 bytes) was 54 bytes ago --- */
u8 efuse_eeprom_data[256]; /* 246 256 */
/* XXX 2 bytes hole, try to pack */
/* --- cacheline 7 boundary (448 bytes) was 56 bytes ago --- */
enum RT_CUSTOMER_ID CustomerID; /* 504 4 */
/* size: 508, cachelines: 8, members: 12 */
/* sum members: 506, holes: 1, sum holes: 2 */
/* last cacheline: 60 bytes */
};
thanks!
Signature · 2 lines
Jagan Teki, Openedev.
#include <stdio.h>
struct foo {
unsigned char one;
unsigned char two;
unsigned char three;
unsigned char mac_addr[6];
unsigned short channel_plan;
};int main(void)
{
struct foo *p = NULL;
printf("%x\n", &p->mac_addr);
return 0;
}This program does not print an even number so the alignment is off.
regards,
dan carpenter