Re: [PATCH 1/6] staging: r8188eu: Replace wrapper around _rtw_memcmp()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I dropped netdev from the mailing list.  Wireless patches generally
should go through linux-wireless instead of netdev anyway.  They get
picked up from there and forwarded on to Dave Miller.

I reviewed these patches when they were posted to the list but I missed
this style issue which Smatch complains about.

On Sun, Feb 09, 2014 at 03:15:54PM -0600, Larry Finger wrote:
>  int rtw_get_wpa_cipher_suite(u8 *s)
>  {
> -	if (_rtw_memcmp(s, WPA_CIPHER_SUITE_NONE, WPA_SELECTOR_LEN) == true)
> +	if (!memcmp(s, WPA_CIPHER_SUITE_NONE, WPA_SELECTOR_LEN) == true)
>  		return WPA_CIPHER_NONE;
> -	if (_rtw_memcmp(s, WPA_CIPHER_SUITE_WEP40, WPA_SELECTOR_LEN) == true)
> +	if (!memcmp(s, WPA_CIPHER_SUITE_WEP40, WPA_SELECTOR_LEN) == true)
>  		return WPA_CIPHER_WEP40;
> -	if (_rtw_memcmp(s, WPA_CIPHER_SUITE_TKIP, WPA_SELECTOR_LEN) == true)
> +	if (!memcmp(s, WPA_CIPHER_SUITE_TKIP, WPA_SELECTOR_LEN) == true)
>  		return WPA_CIPHER_TKIP;
> -	if (_rtw_memcmp(s, WPA_CIPHER_SUITE_CCMP, WPA_SELECTOR_LEN) == true)
> +	if (!memcmp(s, WPA_CIPHER_SUITE_CCMP, WPA_SELECTOR_LEN) == true)
>  		return WPA_CIPHER_CCMP;
> -	if (_rtw_memcmp(s, WPA_CIPHER_SUITE_WEP104, WPA_SELECTOR_LEN) == true)
> +	if (!memcmp(s, WPA_CIPHER_SUITE_WEP104, WPA_SELECTOR_LEN) == true)
>  		return WPA_CIPHER_WEP104;

The (!foo == bar) is harmless here, but more often than not that
formulation indicates a precedence bug.

For memcmp() and to a larger extent for strcmp() then my prefered way
of doing it is:

	if (memcmp(a, b, size) != 0) {

Normally, I rant and rave at code which does != 0 but for memcmp() it is
idiomatic *cmp() functions.  Think about it like this:

	if (a == b) {

Translates to:

	if (memcmp(a, b, size) == 0) {

In your head you move the == to the middle so it's obvious a == b.  Or
for strcmp():

	if (strcmp(a, b, size) < 0) {

means that a is less than b alphabetically.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux