On Thu, 2016-04-07 at 15:07 +0530, Shyam Saini wrote:
> Fixed following warning detected by checkpatch.pl:
> warning: Prefer ether_addr_copy() over memcpy() if the Ethernet addressesare
> __aligned(2)
Please don't just do checkpatch cleanups but strive
to make the code more intelligible for humans.
Also, there's a malloc that may fail that writes through
the returned pointer that could be NULL.
The kmalloc/memcpy could be a kmemdup.
It would make the code shorter and clearer to remove
u32 ap_index and use a temporary pointer for all the
last_scanned_shadow[ap_index]. uses instead.
Something like:
---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 54 +++++++++++------------
1 file changed, 25 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 358632b..35696e2 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -339,49 +339,45 @@ static void add_network_to_shadow(struct network_info
*pstrNetworkInfo,
void *user_void, void *pJoinParams)
{
int ap_found = is_network_in_shadow(pstrNetworkInfo, user_void);
- u32 ap_index = 0;
u8 rssi_index = 0;
+ struct network_info *ni;
if (last_scanned_cnt >= MAX_NUM_SCANNED_NETWORKS_SHADOW)
return;
if (ap_found == -1) {
- ap_index = last_scanned_cnt;
+ ni = &last_scanned_shadow[last_scanned_cnt];
last_scanned_cnt++;
} else {
- ap_index = ap_found;
+ ni = &last_scanned_shadow[ap_found];
}
- rssi_index = last_scanned_shadow[ap_index].str_rssi.u8Index;
- last_scanned_shadow[ap_index].str_rssi.as8RSSI[rssi_index++] =
pstrNetworkInfo->rssi;
+ rssi_index = ni->str_rssi.u8Index;
+ ni->str_rssi.as8RSSI[rssi_index++] = pstrNetworkInfo->rssi;
if (rssi_index == NUM_RSSI) {
rssi_index = 0;
- last_scanned_shadow[ap_index].str_rssi.u8Full = 1;
+ ni->str_rssi.u8Full = 1;
}
- last_scanned_shadow[ap_index].str_rssi.u8Index = rssi_index;
- last_scanned_shadow[ap_index].rssi = pstrNetworkInfo->rssi;
- last_scanned_shadow[ap_index].cap_info = pstrNetworkInfo->cap_info;
- last_scanned_shadow[ap_index].ssid_len = pstrNetworkInfo->ssid_len;
- memcpy(last_scanned_shadow[ap_index].ssid,
- pstrNetworkInfo->ssid, pstrNetworkInfo->ssid_len);
- memcpy(last_scanned_shadow[ap_index].bssid,
- pstrNetworkInfo->bssid, ETH_ALEN);
- last_scanned_shadow[ap_index].beacon_period =
pstrNetworkInfo->beacon_period;
- last_scanned_shadow[ap_index].dtim_period =
pstrNetworkInfo->dtim_period;
- last_scanned_shadow[ap_index].ch = pstrNetworkInfo->ch;
- last_scanned_shadow[ap_index].ies_len = pstrNetworkInfo->ies_len;
- last_scanned_shadow[ap_index].tsf_hi = pstrNetworkInfo->tsf_hi;
+ ni->str_rssi.u8Index = rssi_index;
+ ni->rssi = pstrNetworkInfo->rssi;
+ ni->cap_info = pstrNetworkInfo->cap_info;
+ ni->ssid_len = pstrNetworkInfo->ssid_len;
+ memcpy(ni->ssid, pstrNetworkInfo->ssid, pstrNetworkInfo->ssid_len);
+ ether_addr_copy(ni->bssid, pstrNetworkInfo->bssid);
+ ni->beacon_period = pstrNetworkInfo->beacon_period;
+ ni->dtim_period = pstrNetworkInfo->dtim_period;
+ ni->ch = pstrNetworkInfo->ch;
+ ni->ies_len = pstrNetworkInfo->ies_len;
+ ni->tsf_hi = pstrNetworkInfo->tsf_hi;
if (ap_found != -1)
- kfree(last_scanned_shadow[ap_index].ies);
- last_scanned_shadow[ap_index].ies = kmalloc(pstrNetworkInfo->ies_len,
- GFP_KERNEL);
- memcpy(last_scanned_shadow[ap_index].ies,
- pstrNetworkInfo->ies, pstrNetworkInfo->ies_len);
- last_scanned_shadow[ap_index].time_scan = jiffies;
- last_scanned_shadow[ap_index].time_scan_cached = jiffies;
- last_scanned_shadow[ap_index].found = 1;
+ kfree(ni->ies);
+ ni->ies = kmemdup(pstrNetworkInfo->ies, pstrNetworkInfo->ies_len,
+ GFP_KERNEL);
+ ni->time_scan = jiffies;
+ ni->time_scan_cached = jiffies;
+ ni->found = 1;
if (ap_found != -1)
- kfree(last_scanned_shadow[ap_index].join_params);
- last_scanned_shadow[ap_index].join_params = pJoinParams;
+ kfree(ni->join_params);
+ ni->join_params = pJoinParams;
}
static void CfgScanResult(enum scan_event scan_event,
