[PATCH 0/4] Staging: rtl8712: Fix coding style warnings

[PATCH 0/4] Staging: rtl8712: Fix coding style warnings

[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

[PATCH 1/4] Staging: rtl8712: Fix warning prefer eth_broadcast_addr() over memset()

[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

[PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()

[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

Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()

[Larry Finger]

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

Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()

[punit vara]

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 ?

Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()

[Julia Lawall]

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

Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()

[punit vara]

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.

Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()

[punit vara]

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.

Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()

[kbuild test robot]

[-- 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 --]

Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()

[kbuild test robot]

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

Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()

[punit vara]

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.

Re: [PATCH 2/4] Staging: rtl8712: Use ether_addr_equal() over memcmp()

[Joe Perches]

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.

[PATCH 3/4] Staging: rtl8712: Coding style warnings fix for block comments

[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

[PATCH 4/4] Staging: rtl8712: fix warning for placing constant on the right side of test

[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

Re: [PATCH 4/4] Staging: rtl8712: fix warning for placing constant on the right side of test

[Julia Lawall]

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
>
>

Re: [PATCH 4/4] Staging: rtl8712: fix warning for placing constant on the right side of test

[punit vara]

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