* [PATCH 0/4] Staging: rtl8712: Fix coding style warnings
@ 2015-10-10 16:58 Punit Vara
2015-10-10 16:58 ` [PATCH 1/4] Staging: rtl8712: Fix warning prefer eth_broadcast_addr() over memset() Punit Vara
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Punit Vara @ 2015-10-10 16:58 UTC (permalink / raw)
To: Larry.Finger
Cc: florian.c.schilhabel, gregkh, stillcompiling, sudipm.mukherjee,
dan.carpenter, dogukan.ergun, Julia.Lawall, devel, linux-kernel,
Punit Vara
This set of patch fixes following warnings reported by checkpatch.pl:
-Prefer eth_broadcast_addr() over memset()
-Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()
-Block comments use * on subsequent lines
-Block comments use a trailing */ on a separate line
-Comparisons should place the constant on the right side of the test
Punit Vara (4):
Staging: rtl8712: Fix warning prefer eth_broadcast_addr() over
memset()
Staging: rtl8712: Use ether_addr_equal() over memcmp()
Staging: rtl8712: Coding style warnings fix for block comments
Staging: rtl8712: fix warning for placing constant on the right side
of test
drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 47 ++++++++++++++-------------
1 file changed, 25 insertions(+), 22 deletions(-)
--
2.5.3
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/4] Staging: rtl8712: Fix warning prefer eth_broadcast_addr() over memset()
2015-10-10 16:58 [PATCH 0/4] Staging: rtl8712: Fix coding style warnings Punit Vara
@ 2015-10-10 16:58 ` Punit Vara
2015-10-10 16:58 ` [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp() Punit Vara
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Punit Vara @ 2015-10-10 16:58 UTC (permalink / raw)
To: Larry.Finger
Cc: florian.c.schilhabel, gregkh, stillcompiling, sudipm.mukherjee,
dan.carpenter, dogukan.ergun, Julia.Lawall, devel, linux-kernel,
Punit Vara
This patch is to the rtl871x_ioctl_linux.c that fixes up following
warning by checkpatch.pl:
-Prefer eth_broadcast_addr() over memset()
Signed-off-by: Punit Vara <punitvara@gmail.com>
---
drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 143be0f..2ba055d 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -1811,8 +1811,7 @@ static int r871x_wx_set_enc_ext(struct net_device *dev,
if (param == NULL)
return -ENOMEM;
param->cmd = IEEE_CMD_SET_ENCRYPTION;
- memset(param->sta_addr, 0xff, ETH_ALEN);
-
+ eth_broadcast_addr(param->sta_addr);
strncpy((char *)param->u.crypt.alg, alg_name, IEEE_CRYPT_ALG_NAME_LEN);
if (pext->ext_flags & IW_ENCODE_EXT_GROUP_KEY)
param->u.crypt.set_tx = 0;
--
2.5.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()
2015-10-10 16:58 [PATCH 0/4] Staging: rtl8712: Fix coding style warnings Punit Vara
2015-10-10 16:58 ` [PATCH 1/4] Staging: rtl8712: Fix warning prefer eth_broadcast_addr() over memset() Punit Vara
@ 2015-10-10 16:58 ` Punit Vara
2015-10-10 17:34 ` Larry Finger
` (2 more replies)
2015-10-10 16:58 ` [PATCH 3/4] Staging: rtl8712: Coding style warnings fix for block comments Punit Vara
2015-10-10 16:58 ` [PATCH 4/4] Staging: rtl8712: fix warning for placing constant on the right side of test Punit Vara
3 siblings, 3 replies; 16+ messages in thread
From: Punit Vara @ 2015-10-10 16:58 UTC (permalink / raw)
To: Larry.Finger
Cc: florian.c.schilhabel, gregkh, stillcompiling, sudipm.mukherjee,
dan.carpenter, dogukan.ergun, Julia.Lawall, devel, linux-kernel,
Punit Vara
This patch is to the rtl871x_ioctl_linux.c file that fixes up following
warning reported by checkpatch.pl :
-Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()
bssid has datatype u8 and pnetwork->network.MacAddress has data type
unsigned char.
Signed-off-by: Punit Vara <punitvara@gmail.com>
---
drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 2ba055d..1c9092e 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -2007,7 +2007,7 @@ static int r871x_get_ap_info(struct net_device *dev,
return -EINVAL;
}
netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
- if (!memcmp(bssid, pnetwork->network.MacAddress, ETH_ALEN)) {
+ if (!ether_addr_equal(bssid, pnetwork->network)) {
/* BSSID match, then check if supporting wpa/wpa2 */
pbuf = r8712_get_wpa_ie(&pnetwork->network.IEs[12],
&wpa_ielen, pnetwork->network.IELength-12);
--
2.5.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/4] Staging: rtl8712: Coding style warnings fix for block comments
2015-10-10 16:58 [PATCH 0/4] Staging: rtl8712: Fix coding style warnings Punit Vara
2015-10-10 16:58 ` [PATCH 1/4] Staging: rtl8712: Fix warning prefer eth_broadcast_addr() over memset() Punit Vara
2015-10-10 16:58 ` [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp() Punit Vara
@ 2015-10-10 16:58 ` Punit Vara
2015-10-10 16:58 ` [PATCH 4/4] Staging: rtl8712: fix warning for placing constant on the right side of test Punit Vara
3 siblings, 0 replies; 16+ messages in thread
From: Punit Vara @ 2015-10-10 16:58 UTC (permalink / raw)
To: Larry.Finger
Cc: florian.c.schilhabel, gregkh, stillcompiling, sudipm.mukherjee,
dan.carpenter, dogukan.ergun, Julia.Lawall, devel, linux-kernel,
Punit Vara
This patch is to the rtl871x_ioctl_linux.c file that fixes up following
warnings reported by checkpatch.pl :
-Block comments use * on subsequent lines
-Block comments use a trailing */ on a separate line
Signed-off-by: Punit Vara <punitvara@gmail.com>
---
drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 1c9092e..fc1028a 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -797,13 +797,13 @@ static int r871x_wx_set_pmkid(struct net_device *dev,
int intReturn = false;
/*
- There are the BSSID information in the bssid.sa_data array.
- If cmd is IW_PMKSA_FLUSH, it means the wpa_supplicant wants to clear
- all the PMKID information. If cmd is IW_PMKSA_ADD, it means the
- wpa_supplicant wants to add a PMKID/BSSID to driver.
- If cmd is IW_PMKSA_REMOVE, it means the wpa_supplicant wants to
- remove a PMKID/BSSID from driver.
-*/
+ * There are the BSSID information in the bssid.sa_data array.
+ * If cmd is IW_PMKSA_FLUSH, it means the wpa_supplicant wants to clear
+ * all the PMKID information. If cmd is IW_PMKSA_ADD, it means the
+ * wpa_supplicant wants to add a PMKID/BSSID to driver.
+ * If cmd is IW_PMKSA_REMOVE, it means the wpa_supplicant wants to
+ * remove a PMKID/BSSID from driver.
+ */
if (pPMK == NULL)
return -EINVAL;
memcpy(strIssueBssid, pPMK->bssid.sa_data, ETH_ALEN);
@@ -818,7 +818,8 @@ static int r871x_wx_set_pmkid(struct net_device *dev,
if (!memcmp(psecuritypriv->PMKIDList[j].Bssid,
strIssueBssid, ETH_ALEN)) {
/* BSSID is matched, the same AP => rewrite
- * with new PMKID. */
+ * with new PMKID.
+ */
netdev_info(dev, "r8712u: %s: BSSID exists in the PMKList.\n",
__func__);
memcpy(psecuritypriv->PMKIDList[j].PMKID,
@@ -850,7 +851,8 @@ static int r871x_wx_set_pmkid(struct net_device *dev,
if (!memcmp(psecuritypriv->PMKIDList[j].Bssid,
strIssueBssid, ETH_ALEN)) {
/* BSSID is matched, the same AP => Remove
- * this PMKID information and reset it. */
+ * this PMKID information and reset it.
+ */
eth_zero_addr(psecuritypriv->PMKIDList[j].Bssid);
psecuritypriv->PMKIDList[j].bUsed = false;
break;
@@ -1587,7 +1589,8 @@ static int r8711_wx_set_enc(struct net_device *dev,
} else {
wep.KeyLength = 0;
if (keyindex_provided == 1) { /* set key_id only, no given
- * KeyMaterial(erq->length==0).*/
+ * KeyMaterial(erq->length==0).
+ */
padapter->securitypriv.PrivacyKeyIndex = key;
switch (padapter->securitypriv.DefKeylen[key]) {
case 5:
@@ -2240,7 +2243,8 @@ static iw_handler r8711_handlers[] = {
r8711_wx_set_wap, /* SIOCSIWAP */
r8711_wx_get_wap, /* SIOCGIWAP */
r871x_wx_set_mlme, /* request MLME operation;
- * uses struct iw_mlme */
+ * uses struct iw_mlme
+ */
dummy, /* SIOCGIWAPLIST -- deprecated */
r8711_wx_set_scan, /* SIOCSIWSCAN */
r8711_wx_get_scan, /* SIOCGIWSCAN */
--
2.5.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/4] Staging: rtl8712: fix warning for placing constant on the right side of test
2015-10-10 16:58 [PATCH 0/4] Staging: rtl8712: Fix coding style warnings Punit Vara
` (2 preceding siblings ...)
2015-10-10 16:58 ` [PATCH 3/4] Staging: rtl8712: Coding style warnings fix for block comments Punit Vara
@ 2015-10-10 16:58 ` Punit Vara
2015-10-10 17:07 ` Julia Lawall
3 siblings, 1 reply; 16+ messages in thread
From: Punit Vara @ 2015-10-10 16:58 UTC (permalink / raw)
To: Larry.Finger
Cc: florian.c.schilhabel, gregkh, stillcompiling, sudipm.mukherjee,
dan.carpenter, dogukan.ergun, Julia.Lawall, devel, linux-kernel,
Punit Vara
This patch is to the rtl871x_ioctl_linux.c file that fixes up following
warnings reported by checkpatch.pl:
-Comparisons should place the constant on the right side of the test
Signed-off-by: Punit Vara <punitvara@gmail.com>
---
drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index fc1028a..e9852ae 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -959,7 +959,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
if (IS_ERR(ext))
return PTR_ERR(ext);
- if (0 == strcasecmp(ext, "RSSI")) {
+ if (strcasecmp(ext, "RSSI") == 0) {
/*Return received signal strength indicator in -db for */
/* current AP */
/*<ssid> Rssi xx */
@@ -976,7 +976,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
} else {
sprintf(ext, "OK");
}
- } else if (0 == strcasecmp(ext, "LINKSPEED")) {
+ } else if (strcasecmp(ext, "LINKSPEED") == 0) {
/*Return link speed in MBPS */
/*LinkSpeed xx */
union iwreq_data wrqd;
@@ -984,30 +984,30 @@ static int r871x_wx_set_priv(struct net_device *dev,
int mbps;
ret_inner = r8711_wx_get_rate(dev, info, &wrqd, extra);
- if (0 != ret_inner)
+ if (ret_inner != 0)
mbps = 0;
else
mbps = wrqd.bitrate.value / 1000000;
sprintf(ext, "LINKSPEED %d", mbps);
- } else if (0 == strcasecmp(ext, "MACADDR")) {
+ } else if (strcasecmp(ext, "MACADDR") == 0) {
/*Return mac address of the station */
/* Macaddr = xx:xx:xx:xx:xx:xx */
sprintf(ext, "MACADDR = %pM", dev->dev_addr);
- } else if (0 == strcasecmp(ext, "SCAN-ACTIVE")) {
+ } else if (strcasecmp(ext, "SCAN-ACTIVE") == 0) {
/*Set scan type to active */
/*OK if successful */
struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
pmlmepriv->passive_mode = 1;
sprintf(ext, "OK");
- } else if (0 == strcasecmp(ext, "SCAN-PASSIVE")) {
+ } else if (strcasecmp(ext, "SCAN-PASSIVE") == 0) {
/*Set scan type to passive */
/*OK if successful */
struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
pmlmepriv->passive_mode = 0;
sprintf(ext, "OK");
- } else if (0 == strncmp(ext, "DCE-E", 5)) {
+ } else if (strncmp(ext, "DCE-E", 5) == 0) {
/*Set scan type to passive */
/*OK if successful */
r8712_disconnectCtrlEx_cmd(padapter
@@ -1017,7 +1017,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
, 5000 /*u32 firstStageTO */
);
sprintf(ext, "OK");
- } else if (0 == strncmp(ext, "DCE-D", 5)) {
+ } else if (strncmp(ext, "DCE-D", 5) == 0) {
/*Set scan type to passive */
/*OK if successfu */
r8712_disconnectCtrlEx_cmd(padapter
--
2.5.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] Staging: rtl8712: fix warning for placing constant on the right side of test
2015-10-10 16:58 ` [PATCH 4/4] Staging: rtl8712: fix warning for placing constant on the right side of test Punit Vara
@ 2015-10-10 17:07 ` Julia Lawall
2015-10-10 19:52 ` punit vara
0 siblings, 1 reply; 16+ messages in thread
From: Julia Lawall @ 2015-10-10 17:07 UTC (permalink / raw)
To: Punit Vara
Cc: Larry.Finger, florian.c.schilhabel, gregkh, stillcompiling,
sudipm.mukherjee, dan.carpenter, dogukan.ergun, Julia.Lawall,
devel, linux-kernel
On Sat, 10 Oct 2015, Punit Vara wrote:
> This patch is to the rtl871x_ioctl_linux.c file that fixes up following
The entire above line doesn't give any information (except the file name,
but that can easily be seen from what comes just below). A commit message
that would get to the point (what you did) more quickly could be something
like: "Put constant on the right side of a test. Problem found using
checkpatch."
julia
> warnings reported by checkpatch.pl:
>
> -Comparisons should place the constant on the right side of the test
>
> Signed-off-by: Punit Vara <punitvara@gmail.com>
> ---
> drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index fc1028a..e9852ae 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -959,7 +959,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
> if (IS_ERR(ext))
> return PTR_ERR(ext);
>
> - if (0 == strcasecmp(ext, "RSSI")) {
> + if (strcasecmp(ext, "RSSI") == 0) {
> /*Return received signal strength indicator in -db for */
> /* current AP */
> /*<ssid> Rssi xx */
> @@ -976,7 +976,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
> } else {
> sprintf(ext, "OK");
> }
> - } else if (0 == strcasecmp(ext, "LINKSPEED")) {
> + } else if (strcasecmp(ext, "LINKSPEED") == 0) {
> /*Return link speed in MBPS */
> /*LinkSpeed xx */
> union iwreq_data wrqd;
> @@ -984,30 +984,30 @@ static int r871x_wx_set_priv(struct net_device *dev,
> int mbps;
>
> ret_inner = r8711_wx_get_rate(dev, info, &wrqd, extra);
> - if (0 != ret_inner)
> + if (ret_inner != 0)
> mbps = 0;
> else
> mbps = wrqd.bitrate.value / 1000000;
> sprintf(ext, "LINKSPEED %d", mbps);
> - } else if (0 == strcasecmp(ext, "MACADDR")) {
> + } else if (strcasecmp(ext, "MACADDR") == 0) {
> /*Return mac address of the station */
> /* Macaddr = xx:xx:xx:xx:xx:xx */
> sprintf(ext, "MACADDR = %pM", dev->dev_addr);
> - } else if (0 == strcasecmp(ext, "SCAN-ACTIVE")) {
> + } else if (strcasecmp(ext, "SCAN-ACTIVE") == 0) {
> /*Set scan type to active */
> /*OK if successful */
> struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>
> pmlmepriv->passive_mode = 1;
> sprintf(ext, "OK");
> - } else if (0 == strcasecmp(ext, "SCAN-PASSIVE")) {
> + } else if (strcasecmp(ext, "SCAN-PASSIVE") == 0) {
> /*Set scan type to passive */
> /*OK if successful */
> struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>
> pmlmepriv->passive_mode = 0;
> sprintf(ext, "OK");
> - } else if (0 == strncmp(ext, "DCE-E", 5)) {
> + } else if (strncmp(ext, "DCE-E", 5) == 0) {
> /*Set scan type to passive */
> /*OK if successful */
> r8712_disconnectCtrlEx_cmd(padapter
> @@ -1017,7 +1017,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
> , 5000 /*u32 firstStageTO */
> );
> sprintf(ext, "OK");
> - } else if (0 == strncmp(ext, "DCE-D", 5)) {
> + } else if (strncmp(ext, "DCE-D", 5) == 0) {
> /*Set scan type to passive */
> /*OK if successfu */
> r8712_disconnectCtrlEx_cmd(padapter
> --
> 2.5.3
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()
2015-10-10 16:58 ` [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp() Punit Vara
@ 2015-10-10 17:34 ` Larry Finger
2015-10-10 19:47 ` punit vara
2015-10-10 23:59 ` kbuild test robot
2015-10-11 0:37 ` kbuild test robot
2 siblings, 1 reply; 16+ messages in thread
From: Larry Finger @ 2015-10-10 17:34 UTC (permalink / raw)
To: Punit Vara
Cc: gregkh, stillcompiling, sudipm.mukherjee, dan.carpenter,
dogukan.ergun, Julia.Lawall, devel, linux-kernel
On 10/10/2015 11:58 AM, Punit Vara wrote:
> This patch is to the rtl871x_ioctl_linux.c file that fixes up following
> warning reported by checkpatch.pl :
>
> -Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()
>
> bssid has datatype u8 and pnetwork->network.MacAddress has data type
> unsigned char.
>
> Signed-off-by: Punit Vara <punitvara@gmail.com>
> ---
> drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index 2ba055d..1c9092e 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -2007,7 +2007,7 @@ static int r871x_get_ap_info(struct net_device *dev,
> return -EINVAL;
> }
> netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
> - if (!memcmp(bssid, pnetwork->network.MacAddress, ETH_ALEN)) {
> + if (!ether_addr_equal(bssid, pnetwork->network)) {
> /* BSSID match, then check if supporting wpa/wpa2 */
> pbuf = r8712_get_wpa_ie(&pnetwork->network.IEs[12],
> &wpa_ielen, pnetwork->network.IELength-12);
>
The types of the variables are not what is important - it is the alignment!
I suggest that you read the definition of ether_addr_equal() and consider what
happens if either of the two addresses is not aligned to u16!
You also have a logic error. Routine memcmp() returns zero when the two
arguments are equal. Thus !memcmp() will be true when they are equal. Routine
ether_addr_equal() returns true when the arguments are equal. Whenever !memcmp()
is correct and the arguments are aligned, then you need to replace it with
ether_addr_equal().
The checkpatch script is a good tool for locating things to be changed; however,
if the change is other than cosmetic, you *MUST THINK* about what is happening.
Yes, rtl8712 is ugly code, but it works. Please do not break it.
NACK.
Larry
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()
2015-10-10 17:34 ` Larry Finger
@ 2015-10-10 19:47 ` punit vara
2015-10-10 19:50 ` Julia Lawall
0 siblings, 1 reply; 16+ messages in thread
From: punit vara @ 2015-10-10 19:47 UTC (permalink / raw)
To: Larry Finger
Cc: Greg KH, Joshua Clayton, Sudip Mukherjee, Dan Carpenter,
Dogukan Ergun, Julia.Lawall, devel, linux-kernel
On Sat, Oct 10, 2015 at 11:04 PM, Larry Finger
<Larry.Finger@lwfinger.net> wrote:
> On 10/10/2015 11:58 AM, Punit Vara wrote:
>>
>> This patch is to the rtl871x_ioctl_linux.c file that fixes up following
>> warning reported by checkpatch.pl :
>>
>> -Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()
>>
>> bssid has datatype u8 and pnetwork->network.MacAddress has data type
>> unsigned char.
>>
>> Signed-off-by: Punit Vara <punitvara@gmail.com>
>> ---
>> drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> index 2ba055d..1c9092e 100644
>> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> @@ -2007,7 +2007,7 @@ static int r871x_get_ap_info(struct net_device *dev,
>> return -EINVAL;
>> }
>> netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
>> - if (!memcmp(bssid, pnetwork->network.MacAddress,
>> ETH_ALEN)) {
>> + if (!ether_addr_equal(bssid, pnetwork->network)) {
>> /* BSSID match, then check if supporting wpa/wpa2
>> */
>> pbuf =
>> r8712_get_wpa_ie(&pnetwork->network.IEs[12],
>> &wpa_ielen, pnetwork->network.IELength-12);
>>
>
> The types of the variables are not what is important - it is the alignment!
>
> I suggest that you read the definition of ether_addr_equal() and consider
> what happens if either of the two addresses is not aligned to u16!
>
> You also have a logic error. Routine memcmp() returns zero when the two
> arguments are equal. Thus !memcmp() will be true when they are equal.
> Routine ether_addr_equal() returns true when the arguments are equal.
> Whenever !memcmp() is correct and the arguments are aligned, then you need
> to replace it with ether_addr_equal().
>
> The checkpatch script is a good tool for locating things to be changed;
> however, if the change is other than cosmetic, you *MUST THINK* about what
> is happening. Yes, rtl8712 is ugly code, but it works. Please do not break
> it.
>
> NACK.
>
> Larry
>
@Larry I am thankful for your high quality feedback.
if both address are aligned to u16 then we should use
ether_addr_equal() if not then ether_addr_equal_unaligned().
Both return true if addr1 and addr2 matches. If I am understand
correctly bssid and MacAdress is not aligned to u16 .So right
correction would be
if(ether_addr_equal_unaligned(bssid, pnetwork->network))
right ?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()
2015-10-10 19:47 ` punit vara
@ 2015-10-10 19:50 ` Julia Lawall
2015-10-10 20:22 ` punit vara
0 siblings, 1 reply; 16+ messages in thread
From: Julia Lawall @ 2015-10-10 19:50 UTC (permalink / raw)
To: punit vara
Cc: Larry Finger, Greg KH, Joshua Clayton, Sudip Mukherjee,
Dan Carpenter, Dogukan Ergun, devel, linux-kernel
On Sun, 11 Oct 2015, punit vara wrote:
> On Sat, Oct 10, 2015 at 11:04 PM, Larry Finger
> <Larry.Finger@lwfinger.net> wrote:
> > On 10/10/2015 11:58 AM, Punit Vara wrote:
> >>
> >> This patch is to the rtl871x_ioctl_linux.c file that fixes up following
> >> warning reported by checkpatch.pl :
> >>
> >> -Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()
> >>
> >> bssid has datatype u8 and pnetwork->network.MacAddress has data type
> >> unsigned char.
> >>
> >> Signed-off-by: Punit Vara <punitvara@gmail.com>
> >> ---
> >> drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >> b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >> index 2ba055d..1c9092e 100644
> >> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >> @@ -2007,7 +2007,7 @@ static int r871x_get_ap_info(struct net_device *dev,
> >> return -EINVAL;
> >> }
> >> netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
> >> - if (!memcmp(bssid, pnetwork->network.MacAddress,
> >> ETH_ALEN)) {
> >> + if (!ether_addr_equal(bssid, pnetwork->network)) {
> >> /* BSSID match, then check if supporting wpa/wpa2
> >> */
> >> pbuf =
> >> r8712_get_wpa_ie(&pnetwork->network.IEs[12],
> >> &wpa_ielen, pnetwork->network.IELength-12);
> >>
> >
> > The types of the variables are not what is important - it is the alignment!
> >
> > I suggest that you read the definition of ether_addr_equal() and consider
> > what happens if either of the two addresses is not aligned to u16!
> >
> > You also have a logic error. Routine memcmp() returns zero when the two
> > arguments are equal. Thus !memcmp() will be true when they are equal.
> > Routine ether_addr_equal() returns true when the arguments are equal.
> > Whenever !memcmp() is correct and the arguments are aligned, then you need
> > to replace it with ether_addr_equal().
> >
> > The checkpatch script is a good tool for locating things to be changed;
> > however, if the change is other than cosmetic, you *MUST THINK* about what
> > is happening. Yes, rtl8712 is ugly code, but it works. Please do not break
> > it.
> >
> > NACK.
> >
> > Larry
> >
>
> @Larry I am thankful for your high quality feedback.
> if both address are aligned to u16 then we should use
> ether_addr_equal() if not then ether_addr_equal_unaligned().
>
> Both return true if addr1 and addr2 matches. If I am understand
> correctly bssid and MacAdress is not aligned to u16 .So right
> correction would be
> if(ether_addr_equal_unaligned(bssid, pnetwork->network))
>
> right ?
You can use pahole to check the alignment. Use git log to find other
patches that mention the use of this tool to see what information to
provide in the commit message.
julia
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] Staging: rtl8712: fix warning for placing constant on the right side of test
2015-10-10 17:07 ` Julia Lawall
@ 2015-10-10 19:52 ` punit vara
0 siblings, 0 replies; 16+ messages in thread
From: punit vara @ 2015-10-10 19:52 UTC (permalink / raw)
To: Julia Lawall
Cc: Larry Finger, Florian Schilhabel, Greg KH, Joshua Clayton,
Sudip Mukherjee, Dan Carpenter, Dogukan Ergun, devel,
linux-kernel
On Sat, Oct 10, 2015 at 10:37 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sat, 10 Oct 2015, Punit Vara wrote:
>
>> This patch is to the rtl871x_ioctl_linux.c file that fixes up following
>
> The entire above line doesn't give any information (except the file name,
> but that can easily be seen from what comes just below). A commit message
> that would get to the point (what you did) more quickly could be something
> like: "Put constant on the right side of a test. Problem found using
> checkpatch."
>
> julia
>
>> warnings reported by checkpatch.pl:
>>
>> -Comparisons should place the constant on the right side of the test
>>
>> Signed-off-by: Punit Vara <punitvara@gmail.com>
>> ---
>> drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> index fc1028a..e9852ae 100644
>> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> @@ -959,7 +959,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
>> if (IS_ERR(ext))
>> return PTR_ERR(ext);
>>
>> - if (0 == strcasecmp(ext, "RSSI")) {
>> + if (strcasecmp(ext, "RSSI") == 0) {
>> /*Return received signal strength indicator in -db for */
>> /* current AP */
>> /*<ssid> Rssi xx */
>> @@ -976,7 +976,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
>> } else {
>> sprintf(ext, "OK");
>> }
>> - } else if (0 == strcasecmp(ext, "LINKSPEED")) {
>> + } else if (strcasecmp(ext, "LINKSPEED") == 0) {
>> /*Return link speed in MBPS */
>> /*LinkSpeed xx */
>> union iwreq_data wrqd;
>> @@ -984,30 +984,30 @@ static int r871x_wx_set_priv(struct net_device *dev,
>> int mbps;
>>
>> ret_inner = r8711_wx_get_rate(dev, info, &wrqd, extra);
>> - if (0 != ret_inner)
>> + if (ret_inner != 0)
>> mbps = 0;
>> else
>> mbps = wrqd.bitrate.value / 1000000;
>> sprintf(ext, "LINKSPEED %d", mbps);
>> - } else if (0 == strcasecmp(ext, "MACADDR")) {
>> + } else if (strcasecmp(ext, "MACADDR") == 0) {
>> /*Return mac address of the station */
>> /* Macaddr = xx:xx:xx:xx:xx:xx */
>> sprintf(ext, "MACADDR = %pM", dev->dev_addr);
>> - } else if (0 == strcasecmp(ext, "SCAN-ACTIVE")) {
>> + } else if (strcasecmp(ext, "SCAN-ACTIVE") == 0) {
>> /*Set scan type to active */
>> /*OK if successful */
>> struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>>
>> pmlmepriv->passive_mode = 1;
>> sprintf(ext, "OK");
>> - } else if (0 == strcasecmp(ext, "SCAN-PASSIVE")) {
>> + } else if (strcasecmp(ext, "SCAN-PASSIVE") == 0) {
>> /*Set scan type to passive */
>> /*OK if successful */
>> struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>>
>> pmlmepriv->passive_mode = 0;
>> sprintf(ext, "OK");
>> - } else if (0 == strncmp(ext, "DCE-E", 5)) {
>> + } else if (strncmp(ext, "DCE-E", 5) == 0) {
>> /*Set scan type to passive */
>> /*OK if successful */
>> r8712_disconnectCtrlEx_cmd(padapter
>> @@ -1017,7 +1017,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
>> , 5000 /*u32 firstStageTO */
>> );
>> sprintf(ext, "OK");
>> - } else if (0 == strncmp(ext, "DCE-D", 5)) {
>> + } else if (strncmp(ext, "DCE-D", 5) == 0) {
>> /*Set scan type to passive */
>> /*OK if successfu */
>> r8712_disconnectCtrlEx_cmd(padapter
>> --
>> 2.5.3
>>
>>
I thought warning is clearly showing that constant should be on right
side that's why I did not include any extra detail just include one
warning in patch. But surely from next time I will follow your as per
your valuable feedback.
Thanks
Punit Vara
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()
2015-10-10 19:50 ` Julia Lawall
@ 2015-10-10 20:22 ` punit vara
2015-10-10 20:26 ` punit vara
0 siblings, 1 reply; 16+ messages in thread
From: punit vara @ 2015-10-10 20:22 UTC (permalink / raw)
To: Julia Lawall
Cc: Larry Finger, Greg KH, Joshua Clayton, Sudip Mukherjee,
Dan Carpenter, Dogukan Ergun, devel, linux-kernel
On Sun, Oct 11, 2015 at 1:20 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sun, 11 Oct 2015, punit vara wrote:
>
>> On Sat, Oct 10, 2015 at 11:04 PM, Larry Finger
>> <Larry.Finger@lwfinger.net> wrote:
>> > On 10/10/2015 11:58 AM, Punit Vara wrote:
>> >>
>> >> This patch is to the rtl871x_ioctl_linux.c file that fixes up following
>> >> warning reported by checkpatch.pl :
>> >>
>> >> -Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()
>> >>
>> >> bssid has datatype u8 and pnetwork->network.MacAddress has data type
>> >> unsigned char.
>> >>
>> >> Signed-off-by: Punit Vara <punitvara@gmail.com>
>> >> ---
>> >> drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> >> b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> >> index 2ba055d..1c9092e 100644
>> >> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> >> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> >> @@ -2007,7 +2007,7 @@ static int r871x_get_ap_info(struct net_device *dev,
>> >> return -EINVAL;
>> >> }
>> >> netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
>> >> - if (!memcmp(bssid, pnetwork->network.MacAddress,
>> >> ETH_ALEN)) {
>> >> + if (!ether_addr_equal(bssid, pnetwork->network)) {
>> >> /* BSSID match, then check if supporting wpa/wpa2
>> >> */
>> >> pbuf =
>> >> r8712_get_wpa_ie(&pnetwork->network.IEs[12],
>> >> &wpa_ielen, pnetwork->network.IELength-12);
>> >>
>> >
>> > The types of the variables are not what is important - it is the alignment!
>> >
>> > I suggest that you read the definition of ether_addr_equal() and consider
>> > what happens if either of the two addresses is not aligned to u16!
>> >
>> > You also have a logic error. Routine memcmp() returns zero when the two
>> > arguments are equal. Thus !memcmp() will be true when they are equal.
>> > Routine ether_addr_equal() returns true when the arguments are equal.
>> > Whenever !memcmp() is correct and the arguments are aligned, then you need
>> > to replace it with ether_addr_equal().
>> >
>> > The checkpatch script is a good tool for locating things to be changed;
>> > however, if the change is other than cosmetic, you *MUST THINK* about what
>> > is happening. Yes, rtl8712 is ugly code, but it works. Please do not break
>> > it.
>> >
>> > NACK.
>> >
>> > Larry
>> >
>>
>> @Larry I am thankful for your high quality feedback.
>> if both address are aligned to u16 then we should use
>> ether_addr_equal() if not then ether_addr_equal_unaligned().
>>
>> Both return true if addr1 and addr2 matches. If I am understand
>> correctly bssid and MacAdress is not aligned to u16 .So right
>> correction would be
>> if(ether_addr_equal_unaligned(bssid, pnetwork->network))
>>
>> right ?
>
> You can use pahole to check the alignment. Use git log to find other
> patches that mention the use of this tool to see what information to
> provide in the commit message.
>
> julia
Thanks Julia I didn't know about that dwarf utility I will go through
it .sorry I have forgot to write MacAddress.
So here solution could be either
ether_addr_equal(bssid, pnetwork->network.MacAddress) or
ether_addr_equal_unaligned(bssid->network.MacAddress)
I will learn debugging with help of pahole and again send the patch.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()
2015-10-10 20:22 ` punit vara
@ 2015-10-10 20:26 ` punit vara
0 siblings, 0 replies; 16+ messages in thread
From: punit vara @ 2015-10-10 20:26 UTC (permalink / raw)
To: Julia Lawall
Cc: Larry Finger, Greg KH, Joshua Clayton, Sudip Mukherjee,
Dan Carpenter, Dogukan Ergun, devel, linux-kernel
On Sun, Oct 11, 2015 at 1:52 AM, punit vara <punitvara@gmail.com> wrote:
> On Sun, Oct 11, 2015 at 1:20 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>>
>>
>> On Sun, 11 Oct 2015, punit vara wrote:
>>
>>> On Sat, Oct 10, 2015 at 11:04 PM, Larry Finger
>>> <Larry.Finger@lwfinger.net> wrote:
>>> > On 10/10/2015 11:58 AM, Punit Vara wrote:
>>> >>
>>> >> This patch is to the rtl871x_ioctl_linux.c file that fixes up following
>>> >> warning reported by checkpatch.pl :
>>> >>
>>> >> -Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()
>>> >>
>>> >> bssid has datatype u8 and pnetwork->network.MacAddress has data type
>>> >> unsigned char.
>>> >>
>>> >> Signed-off-by: Punit Vara <punitvara@gmail.com>
>>> >> ---
>>> >> drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 2 +-
>>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>>> >> b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>>> >> index 2ba055d..1c9092e 100644
>>> >> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>>> >> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>>> >> @@ -2007,7 +2007,7 @@ static int r871x_get_ap_info(struct net_device *dev,
>>> >> return -EINVAL;
>>> >> }
>>> >> netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
>>> >> - if (!memcmp(bssid, pnetwork->network.MacAddress,
>>> >> ETH_ALEN)) {
>>> >> + if (!ether_addr_equal(bssid, pnetwork->network)) {
>>> >> /* BSSID match, then check if supporting wpa/wpa2
>>> >> */
>>> >> pbuf =
>>> >> r8712_get_wpa_ie(&pnetwork->network.IEs[12],
>>> >> &wpa_ielen, pnetwork->network.IELength-12);
>>> >>
>>> >
>>> > The types of the variables are not what is important - it is the alignment!
>>> >
>>> > I suggest that you read the definition of ether_addr_equal() and consider
>>> > what happens if either of the two addresses is not aligned to u16!
>>> >
>>> > You also have a logic error. Routine memcmp() returns zero when the two
>>> > arguments are equal. Thus !memcmp() will be true when they are equal.
>>> > Routine ether_addr_equal() returns true when the arguments are equal.
>>> > Whenever !memcmp() is correct and the arguments are aligned, then you need
>>> > to replace it with ether_addr_equal().
>>> >
>>> > The checkpatch script is a good tool for locating things to be changed;
>>> > however, if the change is other than cosmetic, you *MUST THINK* about what
>>> > is happening. Yes, rtl8712 is ugly code, but it works. Please do not break
>>> > it.
>>> >
>>> > NACK.
>>> >
>>> > Larry
>>> >
>>>
>>> @Larry I am thankful for your high quality feedback.
>>> if both address are aligned to u16 then we should use
>>> ether_addr_equal() if not then ether_addr_equal_unaligned().
>>>
>>> Both return true if addr1 and addr2 matches. If I am understand
>>> correctly bssid and MacAdress is not aligned to u16 .So right
>>> correction would be
>>> if(ether_addr_equal_unaligned(bssid, pnetwork->network))
>>>
>>> right ?
>>
>> You can use pahole to check the alignment. Use git log to find other
>> patches that mention the use of this tool to see what information to
>> provide in the commit message.
>>
>> julia
>
> Thanks Julia I didn't know about that dwarf utility I will go through
> it .sorry I have forgot to write MacAddress.
>
> So here solution could be either
>
> ether_addr_equal(bssid, pnetwork->network.MacAddress) or
> ether_addr_equal_unaligned(bssid->network.MacAddress)
>
> I will learn debugging with help of pahole and again send the patch.
Thanks Julia I didn't know about that dwarves utility I will go through
it .sorry I have forgot to write MacAddress.
So here solution could be either
ether_addr_equal(bssid, pnetwork->network.MacAddress) or
ether_addr_equal_unaligned(bssid, pnetwork->network.MacAddress)
I will learn debugging with help of pahole and again send the patch.
Sorry for inconvenience previous mail send before adding because my
laptop was hanged.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()
2015-10-10 16:58 ` [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp() Punit Vara
2015-10-10 17:34 ` Larry Finger
@ 2015-10-10 23:59 ` kbuild test robot
2015-10-11 0:37 ` kbuild test robot
2 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2015-10-10 23:59 UTC (permalink / raw)
To: Punit Vara
Cc: kbuild-all, Larry.Finger, dogukan.ergun, florian.c.schilhabel,
Punit Vara, stillcompiling, gregkh, devel, linux-kernel,
Julia.Lawall, sudipm.mukherjee, dan.carpenter
[-- Attachment #1: Type: text/plain, Size: 4731 bytes --]
Hi Punit,
[auto build test ERROR on v4.3-rc4 -- if it's inappropriate base, please ignore]
config: x86_64-randconfig-x005-201541 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
In file included from include/linux/linkage.h:4:0,
from include/linux/preempt.h:9,
from include/linux/spinlock.h:50,
from drivers/staging/rtl8712/osdep_service.h:32,
from drivers/staging/rtl8712/rtl871x_ioctl_linux.c:32:
drivers/staging/rtl8712/rtl871x_ioctl_linux.c: In function 'r871x_get_ap_info':
>> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:32: error: incompatible type for argument 2 of 'ether_addr_equal'
if (!ether_addr_equal(bssid, pnetwork->network)) {
^
include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
>> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:3: note: in expansion of macro 'if'
if (!ether_addr_equal(bssid, pnetwork->network)) {
^
In file included from drivers/staging/rtl8712/osdep_service.h:39:0,
from drivers/staging/rtl8712/rtl871x_ioctl_linux.c:32:
include/linux/etherdevice.h:310:20: note: expected 'const u8 * {aka const unsigned char *}' but argument is of type 'struct wlan_bssid_ex'
static inline bool ether_addr_equal(const u8 *addr1, const u8 *addr2)
^
In file included from include/linux/linkage.h:4:0,
from include/linux/preempt.h:9,
from include/linux/spinlock.h:50,
from drivers/staging/rtl8712/osdep_service.h:32,
from drivers/staging/rtl8712/rtl871x_ioctl_linux.c:32:
>> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:32: error: incompatible type for argument 2 of 'ether_addr_equal'
if (!ether_addr_equal(bssid, pnetwork->network)) {
^
include/linux/compiler.h:147:40: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
>> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:3: note: in expansion of macro 'if'
if (!ether_addr_equal(bssid, pnetwork->network)) {
^
In file included from drivers/staging/rtl8712/osdep_service.h:39:0,
from drivers/staging/rtl8712/rtl871x_ioctl_linux.c:32:
include/linux/etherdevice.h:310:20: note: expected 'const u8 * {aka const unsigned char *}' but argument is of type 'struct wlan_bssid_ex'
static inline bool ether_addr_equal(const u8 *addr1, const u8 *addr2)
^
In file included from include/linux/linkage.h:4:0,
from include/linux/preempt.h:9,
from include/linux/spinlock.h:50,
from drivers/staging/rtl8712/osdep_service.h:32,
from drivers/staging/rtl8712/rtl871x_ioctl_linux.c:32:
>> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:32: error: incompatible type for argument 2 of 'ether_addr_equal'
if (!ether_addr_equal(bssid, pnetwork->network)) {
^
include/linux/compiler.h:158:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^
>> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:3: note: in expansion of macro 'if'
if (!ether_addr_equal(bssid, pnetwork->network)) {
^
In file included from drivers/staging/rtl8712/osdep_service.h:39:0,
from drivers/staging/rtl8712/rtl871x_ioctl_linux.c:32:
include/linux/etherdevice.h:310:20: note: expected 'const u8 * {aka const unsigned char *}' but argument is of type 'struct wlan_bssid_ex'
static inline bool ether_addr_equal(const u8 *addr1, const u8 *addr2)
^
vim +/ether_addr_equal +2010 drivers/staging/rtl8712/rtl871x_ioctl_linux.c
2004 (u8 *)data);
2005 spin_unlock_irqrestore(&(pmlmepriv->scanned_queue.lock),
2006 irqL);
2007 return -EINVAL;
2008 }
2009 netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
> 2010 if (!ether_addr_equal(bssid, pnetwork->network)) {
2011 /* BSSID match, then check if supporting wpa/wpa2 */
2012 pbuf = r8712_get_wpa_ie(&pnetwork->network.IEs[12],
2013 &wpa_ielen, pnetwork->network.IELength-12);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 31780 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()
2015-10-10 16:58 ` [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp() Punit Vara
2015-10-10 17:34 ` Larry Finger
2015-10-10 23:59 ` kbuild test robot
@ 2015-10-11 0:37 ` kbuild test robot
2015-10-11 11:12 ` punit vara
2 siblings, 1 reply; 16+ messages in thread
From: kbuild test robot @ 2015-10-11 0:37 UTC (permalink / raw)
To: Punit Vara
Cc: kbuild-all, Larry.Finger, dogukan.ergun, florian.c.schilhabel,
Punit Vara, stillcompiling, gregkh, devel, linux-kernel,
Julia.Lawall, sudipm.mukherjee, dan.carpenter
Hi Punit,
[auto build test WARNING on v4.3-rc4 -- if it's inappropriate base, please ignore]
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
drivers/staging/rtl8712/rtl871x_ioctl_linux.c:227:15: sparse: cast to restricted __le16
>> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:54: sparse: incorrect type in argument 2 (different base types)
drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:54: expected unsigned char const [usertype] *addr2
drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:54: got struct wlan_bssid_ex network
drivers/staging/rtl8712/rtl871x_ioctl_linux.c: In function 'r871x_get_ap_info':
drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:32: error: incompatible type for argument 2 of 'ether_addr_equal'
if (!ether_addr_equal(bssid, pnetwork->network)) {
^
In file included from drivers/staging/rtl8712/osdep_service.h:39:0,
from drivers/staging/rtl8712/rtl871x_ioctl_linux.c:32:
include/linux/etherdevice.h:310:20: note: expected 'const u8 * {aka const unsigned char *}' but argument is of type 'struct wlan_bssid_ex'
static inline bool ether_addr_equal(const u8 *addr1, const u8 *addr2)
^
vim +2010 drivers/staging/rtl8712/rtl871x_ioctl_linux.c
1994 return -EINVAL;
1995 spin_lock_irqsave(&(pmlmepriv->scanned_queue.lock), irqL);
1996 phead = &queue->queue;
1997 plist = phead->next;
1998 while (1) {
1999 if (end_of_queue_search(phead, plist) == true)
2000 break;
2001 pnetwork = LIST_CONTAINOR(plist, struct wlan_network, list);
2002 if (hwaddr_aton_i(data, bssid)) {
2003 netdev_info(dev, "r8712u: Invalid BSSID '%s'.\n",
2004 (u8 *)data);
2005 spin_unlock_irqrestore(&(pmlmepriv->scanned_queue.lock),
2006 irqL);
2007 return -EINVAL;
2008 }
2009 netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
> 2010 if (!ether_addr_equal(bssid, pnetwork->network)) {
2011 /* BSSID match, then check if supporting wpa/wpa2 */
2012 pbuf = r8712_get_wpa_ie(&pnetwork->network.IEs[12],
2013 &wpa_ielen, pnetwork->network.IELength-12);
2014 if (pbuf && (wpa_ielen > 0)) {
2015 pdata->flags = 1;
2016 break;
2017 }
2018 pbuf = r8712_get_wpa2_ie(&pnetwork->network.IEs[12],
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()
2015-10-11 0:37 ` kbuild test robot
@ 2015-10-11 11:12 ` punit vara
2015-10-11 17:23 ` Joe Perches
0 siblings, 1 reply; 16+ messages in thread
From: punit vara @ 2015-10-11 11:12 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Larry Finger, Dogukan Ergun, Florian Schilhabel,
Joshua Clayton, Greg KH, devel, linux-kernel, Julia Lawall,
Sudip Mukherjee, Dan Carpenter
On Sun, Oct 11, 2015 at 6:07 AM, kbuild test robot <lkp@intel.com> wrote:
> Hi Punit,
>
> [auto build test WARNING on v4.3-rc4 -- if it's inappropriate base, please ignore]
>
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:227:15: sparse: cast to restricted __le16
>>> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:54: sparse: incorrect type in argument 2 (different base types)
> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:54: expected unsigned char const [usertype] *addr2
> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:54: got struct wlan_bssid_ex network
> drivers/staging/rtl8712/rtl871x_ioctl_linux.c: In function 'r871x_get_ap_info':
> drivers/staging/rtl8712/rtl871x_ioctl_linux.c:2010:32: error: incompatible type for argument 2 of 'ether_addr_equal'
> if (!ether_addr_equal(bssid, pnetwork->network)) {
> ^
> In file included from drivers/staging/rtl8712/osdep_service.h:39:0,
> from drivers/staging/rtl8712/rtl871x_ioctl_linux.c:32:
> include/linux/etherdevice.h:310:20: note: expected 'const u8 * {aka const unsigned char *}' but argument is of type 'struct wlan_bssid_ex'
> static inline bool ether_addr_equal(const u8 *addr1, const u8 *addr2)
> ^
>
> vim +2010 drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>
> 1994 return -EINVAL;
> 1995 spin_lock_irqsave(&(pmlmepriv->scanned_queue.lock), irqL);
> 1996 phead = &queue->queue;
> 1997 plist = phead->next;
> 1998 while (1) {
> 1999 if (end_of_queue_search(phead, plist) == true)
> 2000 break;
> 2001 pnetwork = LIST_CONTAINOR(plist, struct wlan_network, list);
> 2002 if (hwaddr_aton_i(data, bssid)) {
> 2003 netdev_info(dev, "r8712u: Invalid BSSID '%s'.\n",
> 2004 (u8 *)data);
> 2005 spin_unlock_irqrestore(&(pmlmepriv->scanned_queue.lock),
> 2006 irqL);
> 2007 return -EINVAL;
> 2008 }
> 2009 netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
>> 2010 if (!ether_addr_equal(bssid, pnetwork->network)) {
> 2011 /* BSSID match, then check if supporting wpa/wpa2 */
> 2012 pbuf = r8712_get_wpa_ie(&pnetwork->network.IEs[12],
> 2013 &wpa_ielen, pnetwork->network.IELength-12);
> 2014 if (pbuf && (wpa_ielen > 0)) {
> 2015 pdata->flags = 1;
> 2016 break;
> 2017 }
> 2018 pbuf = r8712_get_wpa2_ie(&pnetwork->network.IEs[12],
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
That error is because I forgot MacAddress that I have already mention.
Julia I have used pahole as you suggested me
following are the two structure need to be consider for alignment.
struct iw_pmksa {
__u32 cmd; /* 0 4 */
struct sockaddr bssid; /* 4 16 */
__u8 pmkid[16]; /* 20 16 */
/* size: 36, cachelines: 1, members: 3 */
/* last cacheline: 36 bytes */
};
struct wlan_bssid_ex {
u32 Length; /* 0 4 */
unsigned char MacAddress[6]; /* 4 6 */
u8 Reserved[2]; /* 10 2 */
struct ndis_802_11_ssid Ssid; /* 12 36 */
u32 Privacy; /* 48 4 */
s32 Rssi; /* 52 4 */
enum NDIS_802_11_NETWORK_TYPE NetworkTypeInUse; /* 56 4 */
struct NDIS_802_11_CONFIGURATION Configuration; /* 60 32 */
/* --- cacheline 1 boundary (64 bytes) was 28 bytes ago --- */
enum NDIS_802_11_NETWORK_INFRASTRUCTURE InfrastructureMode; /* 92 4 */
u8 rates[16]; /* 96 16 */
u32 IELength; /* 112 4 */
u8 IEs[768]; /* 116 768 */
/* --- cacheline 13 boundary (832 bytes) was 52 bytes ago --- */
/* size: 884, cachelines: 14, members: 12 */
/* last cacheline: 52 bytes */
};
As I understood both are not aligned to u16 so
ether_addr_equal_unaligned() should be used.
Here ether_addr_equal_unaligned() return true if both address are equal.
so here correct answer would be
if(ether_addr_equal_unaligned(bssid, pnetwork->network.MacAddress)) ??
If I am getting wrong any feedback are welcome otherwise I will
resend patch with correct modification.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()
2015-10-11 11:12 ` punit vara
@ 2015-10-11 17:23 ` Joe Perches
0 siblings, 0 replies; 16+ messages in thread
From: Joe Perches @ 2015-10-11 17:23 UTC (permalink / raw)
To: punit vara
Cc: kbuild test robot, kbuild-all, Larry Finger, Dogukan Ergun,
Florian Schilhabel, Joshua Clayton, Greg KH, devel, linux-kernel,
Julia Lawall, Sudip Mukherjee, Dan Carpenter
On Sun, 2015-10-11 at 16:42 +0530, punit vara wrote:
> following are the two structure need to be consider for alignment.
>
> struct iw_pmksa {
> __u32 cmd; /* 0 4 */
> struct sockaddr bssid; /* 4 16 */
> __u8 pmkid[16]; /* 20 16 */
wrong bssid, bssid here is on the stack
after a char *, so bssid is aligned on a
pointer boundary, either 4 or 8.
> /* size: 36, cachelines: 1, members: 3 */
> /* last cacheline: 36 bytes */
> };
>
> struct wlan_bssid_ex {
[]
> unsigned char MacAddress[6]; /* 4 6 */
> As I understood both are not aligned to u16 so
> ether_addr_equal_unaligned() should be used.
u16s are aligned when on any even address
So ether_addr_equal could be used.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-10-11 17:23 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-10 16:58 [PATCH 0/4] Staging: rtl8712: Fix coding style warnings Punit Vara
2015-10-10 16:58 ` [PATCH 1/4] Staging: rtl8712: Fix warning prefer eth_broadcast_addr() over memset() Punit Vara
2015-10-10 16:58 ` [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp() Punit Vara
2015-10-10 17:34 ` Larry Finger
2015-10-10 19:47 ` punit vara
2015-10-10 19:50 ` Julia Lawall
2015-10-10 20:22 ` punit vara
2015-10-10 20:26 ` punit vara
2015-10-10 23:59 ` kbuild test robot
2015-10-11 0:37 ` kbuild test robot
2015-10-11 11:12 ` punit vara
2015-10-11 17:23 ` Joe Perches
2015-10-10 16:58 ` [PATCH 3/4] Staging: rtl8712: Coding style warnings fix for block comments Punit Vara
2015-10-10 16:58 ` [PATCH 4/4] Staging: rtl8712: fix warning for placing constant on the right side of test Punit Vara
2015-10-10 17:07 ` Julia Lawall
2015-10-10 19:52 ` punit vara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).