On Sat, Oct 10, 2015 at 11:04 PM, Larry Finger
<Larry.Finger@xxxxxxxxxxxx> 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@xxxxxxxxx>
>> ---
>> 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 ?
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
