[PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()

[PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()

[Mao Wenan]

Using is_zero_ether_addr() instead of directly use
memcmp() to determine if the ethernet address is all
zeros.

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 drivers/staging/rtl8188eu/core/rtw_mlme.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index 714f7a70ed64..beae367df93b 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -180,9 +180,8 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
 {
 	struct list_head *phead, *plist;
 	struct wlan_network *pnetwork = NULL;
-	u8 zero_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
 
-	if (!memcmp(zero_addr, addr, ETH_ALEN)) {
+	if (is_zero_ether_addr(addr)) {
 		pnetwork = NULL;
 		goto exit;
 	}
-- 
2.20.1

Re: [PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()

[maowenan]

ping...

On 2019/3/9 11:26, Mao Wenan wrote:
> Using is_zero_ether_addr() instead of directly use
> memcmp() to determine if the ethernet address is all
> zeros.
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  drivers/staging/rtl8188eu/core/rtw_mlme.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> index 714f7a70ed64..beae367df93b 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> @@ -180,9 +180,8 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
>  {
>  	struct list_head *phead, *plist;
>  	struct wlan_network *pnetwork = NULL;
> -	u8 zero_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
>  
> -	if (!memcmp(zero_addr, addr, ETH_ALEN)) {
> +	if (is_zero_ether_addr(addr)) {
>  		pnetwork = NULL;
>  		goto exit;
>  	}
> 

Re: [PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()

[Joe Perches]

On Tue, 2019-03-12 at 14:29 +0800, maowenan wrote:
> ping...
> 
> On 2019/3/9 11:26, Mao Wenan wrote:
> > Using is_zero_ether_addr() instead of directly use
> > memcmp() to determine if the ethernet address is all
> > zeros.
[]
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
[]
> > @@ -180,9 +180,8 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
> >  {
> >  	struct list_head *phead, *plist;
> >  	struct wlan_network *pnetwork = NULL;
> > -	u8 zero_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
> >  
> > -	if (!memcmp(zero_addr, addr, ETH_ALEN)) {
> > +	if (is_zero_ether_addr(addr)) {

How did you verify that addr is __aligned(2)?

Re: [PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()

[maowenan]

On 2019/3/12 14:35, Joe Perches wrote:
> On Tue, 2019-03-12 at 14:29 +0800, maowenan wrote:
>> ping...
>>
>> On 2019/3/9 11:26, Mao Wenan wrote:
>>> Using is_zero_ether_addr() instead of directly use
>>> memcmp() to determine if the ethernet address is all
>>> zeros.
> []
>>> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> []
>>> @@ -180,9 +180,8 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
>>>  {
>>>  	struct list_head *phead, *plist;
>>>  	struct wlan_network *pnetwork = NULL;
>>> -	u8 zero_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
>>>  
>>> -	if (!memcmp(zero_addr, addr, ETH_ALEN)) {
>>> +	if (is_zero_ether_addr(addr)) {
> 
> How did you verify that addr is __aligned(2)?

/**
 * is_zero_ether_addr - Determine if give Ethernet address is all zeros.
 * @addr: Pointer to a six-byte array containing the Ethernet address
 *
 * Return true if the address is all zeroes.
 */
I think they are completely equivalent functions, no need to check addr is __aligned(2),
because addr may be defined as unsigned char MacAddress[ETH_ALEN]; the length is 6.
> 
> 
> 
> .
> 

Re: [PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()

[Joe Perches]

On Tue, 2019-03-12 at 14:58 +0800, maowenan wrote:
> On 2019/3/12 14:35, Joe Perches wrote:
> > On Tue, 2019-03-12 at 14:29 +0800, maowenan wrote:
> > > ping...
> > > 
> > > On 2019/3/9 11:26, Mao Wenan wrote:
> > > > Using is_zero_ether_addr() instead of directly use
> > > > memcmp() to determine if the ethernet address is all
> > > > zeros.
> > []
> > > > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > []
> > > > @@ -180,9 +180,8 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
> > > >  {
> > > >  	struct list_head *phead, *plist;
> > > >  	struct wlan_network *pnetwork = NULL;
> > > > -	u8 zero_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
> > > >  
> > > > -	if (!memcmp(zero_addr, addr, ETH_ALEN)) {
> > > > +	if (is_zero_ether_addr(addr)) {
> > 
> > How did you verify that addr is __aligned(2)?
> 
> /**
>  * is_zero_ether_addr - Determine if give Ethernet address is all zeros.
>  * @addr: Pointer to a six-byte array containing the Ethernet address
>  *
>  * Return true if the address is all zeroes.
>  */
> I think they are completely equivalent functions, no need to check addr is __aligned(2),
> because addr may be defined as unsigned char MacAddress[ETH_ALEN]; the length is 6.

Perhaps you are reading old documentation.
(though the "Please note:" line is from 2013)

include/linux/etherdevice.h-/**
include/linux/etherdevice.h- * is_zero_ether_addr - Determine if give Ethernet address is all zeros.
include/linux/etherdevice.h- * @addr: Pointer to a six-byte array containing the Ethernet address
include/linux/etherdevice.h- *
include/linux/etherdevice.h- * Return true if the address is all zeroes.
include/linux/etherdevice.h- *
include/linux/etherdevice.h- * Please note: addr must be aligned to u16.

Re: [PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()

[Julia Lawall]

On Tue, 12 Mar 2019, maowenan wrote:

>
>
> On 2019/3/12 14:35, Joe Perches wrote:
> > On Tue, 2019-03-12 at 14:29 +0800, maowenan wrote:
> >> ping...
> >>
> >> On 2019/3/9 11:26, Mao Wenan wrote:
> >>> Using is_zero_ether_addr() instead of directly use
> >>> memcmp() to determine if the ethernet address is all
> >>> zeros.
> > []
> >>> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > []
> >>> @@ -180,9 +180,8 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
> >>>  {
> >>>  	struct list_head *phead, *plist;
> >>>  	struct wlan_network *pnetwork = NULL;
> >>> -	u8 zero_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
> >>>
> >>> -	if (!memcmp(zero_addr, addr, ETH_ALEN)) {
> >>> +	if (is_zero_ether_addr(addr)) {
> >
> > How did you verify that addr is __aligned(2)?
>
> /**
>  * is_zero_ether_addr - Determine if give Ethernet address is all zeros.
>  * @addr: Pointer to a six-byte array containing the Ethernet address
>  *
>  * Return true if the address is all zeroes.
>  */
> I think they are completely equivalent functions, no need to check addr is __aligned(2),
> because addr may be defined as unsigned char MacAddress[ETH_ALEN]; the length is 6.

Perhaps you are confusing with eth_zero_addr, which is just a memset and
has no alignment requirements.

julia

Re: [PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()

[Greg KH]

On Tue, Mar 12, 2019 at 02:29:29PM +0800, maowenan wrote:
> ping...
> 
> On 2019/3/9 11:26, Mao Wenan wrote:

What?  You got an automated email from my patch system saying that I can
not do anything with this patch until after 5.1-rc1 is out, due to the
merge window.  Why did that not answer the question ahead of time as to
why this patch was not responded to?

Also, it's only been 3 days, please relax, for a clean-up patch, it can
take weeks before they are reviewed as they are obvious at the bottom of
everyone's priority queue.

so all is good, just wait.

greg k-h

Re: [PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()

[Greg KH]

On Sat, Mar 09, 2019 at 11:26:00AM +0800, Mao Wenan wrote:
> Using is_zero_ether_addr() instead of directly use
> memcmp() to determine if the ethernet address is all
> zeros.
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  drivers/staging/rtl8188eu/core/rtw_mlme.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> index 714f7a70ed64..beae367df93b 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> @@ -180,9 +180,8 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
>  {
>  	struct list_head *phead, *plist;
>  	struct wlan_network *pnetwork = NULL;
> -	u8 zero_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
>  
> -	if (!memcmp(zero_addr, addr, ETH_ALEN)) {
> +	if (is_zero_ether_addr(addr)) {

As Joe said, you have to prove that this is properly aligned before you
can call this function.  Please do so.

thanks,

greg k-h

Re: [PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()

[maowenan]

On 2019/3/17 19:26, Greg KH wrote:
> On Sat, Mar 09, 2019 at 11:26:00AM +0800, Mao Wenan wrote:
>> Using is_zero_ether_addr() instead of directly use
>> memcmp() to determine if the ethernet address is all
>> zeros.
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>>  drivers/staging/rtl8188eu/core/rtw_mlme.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
>> index 714f7a70ed64..beae367df93b 100644
>> --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
>> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
>> @@ -180,9 +180,8 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
>>  {
>>  	struct list_head *phead, *plist;
>>  	struct wlan_network *pnetwork = NULL;
>> -	u8 zero_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
>>  
>> -	if (!memcmp(zero_addr, addr, ETH_ALEN)) {
>> +	if (is_zero_ether_addr(addr)) {
> 
> As Joe said, you have to prove that this is properly aligned before you
> can call this function.  Please do so.

The previous function to call rtw_find_network(), has been aligned for parameter 'addr',
because it has been defined an arry[6];
such as unsigned char MacAddress[ETH_ALEN] in struct wlan_bssid_ex;
 179 struct wlan_bssid_ex {
 180         u32  Length;
 181         unsigned char MacAddress[ETH_ALEN];
 182         u8  Reserved[2];/* 0]: IS beacon frame */
 183         struct ndis_802_11_ssid  ssid;
 184         u32  Privacy;
 185         NDIS_802_11_RSSI  Rssi;/* in dBM,raw data ,get from PHY) */
 186         enum  NDIS_802_11_NETWORK_TYPE  NetworkTypeInUse;
 187         struct ndis_802_11_config  Configuration;
 188         enum ndis_802_11_network_infra  InfrastructureMode;
 189         unsigned char SupportedRates[NDIS_802_11_LENGTH_RATES_EX];
 190         struct wlan_phy_info    PhyInfo;
 191         u32  ie_length;
 192         u8  ies[MAX_IE_SZ];     /* timestamp, beacon interval, and
 193                                  * capability information)
 194                                  */
 195 } __packed;

rtw_survey_event_callback->rtw_find_network(&pmlmepriv->scanned_queue,  pnetwork->MacAddress);
struct wlan_bssid_ex *pnetwork;

> 
> thanks,
> 
> greg k-h
> 
> .
>