[PATCH v2 0/6] Staging: rtl8192e: rtllib_wx codingstyle/refactor

[PATCH v2 0/6] Staging: rtl8192e: rtllib_wx codingstyle/refactor

[Felix Schlepper]

This series addresses some issues raised by chechpatch.pl
and some minor refactoring of rtllib_modes[].

v2:
    - split multi assignment in two separate steps.
      This avoids having 'fixed'='disabled', which
      is silly.
    - Since there is no formatting of the rtllib_modes[],
      one can safely refactor this code into a array of
      strings. This allows using functions like strcpy/strlen,
      which is less error prone than using a
      rtllib_modes.mode_size.

Felix Schlepper (6):
  Staging: rtl8192e: Refactored rtllib_modes
  Staging: rtl8192e: Avoid multiple assignments
  Staging: rtl8192e: Remove unnecessary parentheses
  Staging: rtl8192e: Added braces around else
  Staging: rtl8192e: Remove unnecessary blank line
  Staging: rtl8192e: Added spaces around '+'

 drivers/staging/rtl8192e/rtllib_wx.c | 37 +++++++++++-----------------
 1 file changed, 14 insertions(+), 23 deletions(-)

-- 
2.36.1

[PATCH v2 1/6] Staging: rtl8192e: Refactored rtllib_modes

[Felix Schlepper]

The initial reason for looking at this code was an
issue raised by checkpatch.pl:

     $ ./scripts/checkpatch.pl --terse -f drivers/staging/rtl8192e/rtllib_wx.c
     CHECK: Please use a blank line after function/struct/union/enum
     declarations

The additional blank line above the struct/before the headers is
just cleaner.

However, as it turns out since there is no str formatting required
One can replace the error prone str + size struct with a char array.
The rest of this patch fixes the usecases.

Signed-off-by: Felix Schlepper <f3sch.git@outlook.com>
---
 drivers/staging/rtl8192e/rtllib_wx.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib_wx.c b/drivers/staging/rtl8192e/rtllib_wx.c
index cf9a240924f2..1e420230d029 100644
--- a/drivers/staging/rtl8192e/rtllib_wx.c
+++ b/drivers/staging/rtl8192e/rtllib_wx.c
@@ -17,18 +17,8 @@
 #include <linux/module.h>
 #include <linux/etherdevice.h>
 #include "rtllib.h"
-struct modes_unit {
-	char *mode_string;
-	int mode_size;
-};
-static struct modes_unit rtllib_modes[] = {
-	{"a", 1},
-	{"b", 1},
-	{"g", 1},
-	{"?", 1},
-	{"N-24G", 5},
-	{"N-5G", 4},
-};
+
+static const char *rtllib_modes[] = { "a", "b", "g", "?", "N-24G", "N-5G" };
 
 #define MAX_CUSTOM_LEN 64
 static inline char *rtl819x_translate_scan(struct rtllib_device *ieee,
@@ -72,10 +62,9 @@ static inline char *rtl819x_translate_scan(struct rtllib_device *ieee,
 	/* Add the protocol name */
 	iwe.cmd = SIOCGIWNAME;
 	for (i = 0; i < ARRAY_SIZE(rtllib_modes); i++) {
-		if (network->mode&(1<<i)) {
-			sprintf(pname, rtllib_modes[i].mode_string,
-				rtllib_modes[i].mode_size);
-			pname += rtllib_modes[i].mode_size;
+		if(network->mode & BIT(i)){
+			strcpy(pname, rtllib_modes[i]);
+			pname += strlen(rtllib_modes[i]);
 		}
 	}
 	*pname = '\0';
-- 
2.36.1

Re: [PATCH v2 1/6] Staging: rtl8192e: Refactored rtllib_modes

[Greg KH]

On Thu, Jun 30, 2022 at 01:30:14PM +0200, Felix Schlepper wrote:
> The initial reason for looking at this code was an
> issue raised by checkpatch.pl:
> 
>      $ ./scripts/checkpatch.pl --terse -f drivers/staging/rtl8192e/rtllib_wx.c
>      CHECK: Please use a blank line after function/struct/union/enum
>      declarations
> 
> The additional blank line above the struct/before the headers is
> just cleaner.
> 
> However, as it turns out since there is no str formatting required
> One can replace the error prone str + size struct with a char array.
> The rest of this patch fixes the usecases.
> 
> Signed-off-by: Felix Schlepper <f3sch.git@outlook.com>
> ---
>  drivers/staging/rtl8192e/rtllib_wx.c | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtllib_wx.c b/drivers/staging/rtl8192e/rtllib_wx.c
> index cf9a240924f2..1e420230d029 100644
> --- a/drivers/staging/rtl8192e/rtllib_wx.c
> +++ b/drivers/staging/rtl8192e/rtllib_wx.c
> @@ -17,18 +17,8 @@
>  #include <linux/module.h>
>  #include <linux/etherdevice.h>
>  #include "rtllib.h"
> -struct modes_unit {
> -	char *mode_string;
> -	int mode_size;
> -};
> -static struct modes_unit rtllib_modes[] = {
> -	{"a", 1},
> -	{"b", 1},
> -	{"g", 1},
> -	{"?", 1},
> -	{"N-24G", 5},
> -	{"N-5G", 4},
> -};
> +
> +static const char *rtllib_modes[] = { "a", "b", "g", "?", "N-24G", "N-5G" };
>  
>  #define MAX_CUSTOM_LEN 64
>  static inline char *rtl819x_translate_scan(struct rtllib_device *ieee,
> @@ -72,10 +62,9 @@ static inline char *rtl819x_translate_scan(struct rtllib_device *ieee,
>  	/* Add the protocol name */
>  	iwe.cmd = SIOCGIWNAME;
>  	for (i = 0; i < ARRAY_SIZE(rtllib_modes); i++) {
> -		if (network->mode&(1<<i)) {
> -			sprintf(pname, rtllib_modes[i].mode_string,
> -				rtllib_modes[i].mode_size);
> -			pname += rtllib_modes[i].mode_size;
> +		if(network->mode & BIT(i)){
> +			strcpy(pname, rtllib_modes[i]);
> +			pname += strlen(rtllib_modes[i]);
>  		}
>  	}
>  	*pname = '\0';
> -- 
> 2.36.1
> 
> 

Always run checkpatch on your changes so you do not get grumpy
maintainers asking you why you did not run checkpatch on your changes...

thanks,

greg k-h

[PATCH v2 2/6] Staging: rtl8192e: Avoid multiple assignments

[Felix Schlepper]

This addresses an issue raised by checkpatch.pl:

     $ ./scripts/checkpatch.pl --terse -f drivers/staging/rtl8192e/rtllib_wx.c
     CHECK: multiple assignments should be avoided

Signed-off-by: Felix Schlepper <f3sch.git@outlook.com>
---
 drivers/staging/rtl8192e/rtllib_wx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtllib_wx.c b/drivers/staging/rtl8192e/rtllib_wx.c
index 1e420230d029..f8683b914668 100644
--- a/drivers/staging/rtl8192e/rtllib_wx.c
+++ b/drivers/staging/rtl8192e/rtllib_wx.c
@@ -147,7 +147,8 @@ static inline char *rtl819x_translate_scan(struct rtllib_device *ieee,
 			max_rate = rate;
 	}
 	iwe.cmd = SIOCGIWRATE;
-	iwe.u.bitrate.fixed = iwe.u.bitrate.disabled = 0;
+	iwe.u.bitrate.disabled = 0;
+	iwe.u.bitrate.fixed = 0;
 	iwe.u.bitrate.value = max_rate * 500000;
 	start = iwe_stream_add_event_rsl(info, start, stop, &iwe, IW_EV_PARAM_LEN);
 	iwe.cmd = IWEVCUSTOM;
-- 
2.36.1

[PATCH v2 3/6] Staging: rtl8192e: Remove unnecessary parentheses

[Felix Schlepper]

This addresses an issue raised by checkpatch.pl:

     $ ./scripts/checkpatch.pl --terse -f drivers/staging/rtl8192e/rtllib_wx.c
     Unnecessary parentheses around wrqu->encoding

Signed-off-by: Felix Schlepper <f3sch.git@outlook.com>
---
 drivers/staging/rtl8192e/rtllib_wx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib_wx.c b/drivers/staging/rtl8192e/rtllib_wx.c
index f8683b914668..49b3d4c8f5d6 100644
--- a/drivers/staging/rtl8192e/rtllib_wx.c
+++ b/drivers/staging/rtl8192e/rtllib_wx.c
@@ -275,7 +275,7 @@ int rtllib_wx_set_encode(struct rtllib_device *ieee,
 			 struct iw_request_info *info,
 			 union iwreq_data *wrqu, char *keybuf)
 {
-	struct iw_point *erq = &(wrqu->encoding);
+	struct iw_point *erq = &wrqu->encoding;
 	struct net_device *dev = ieee->dev;
 	struct rtllib_security sec = {
 		.flags = 0
@@ -447,7 +447,7 @@ int rtllib_wx_get_encode(struct rtllib_device *ieee,
 			 struct iw_request_info *info,
 			 union iwreq_data *wrqu, char *keybuf)
 {
-	struct iw_point *erq = &(wrqu->encoding);
+	struct iw_point *erq = &wrqu->encoding;
 	int len, key;
 	struct lib80211_crypt_data *crypt;
 
-- 
2.36.1

[PATCH v2 4/6] Staging: rtl8192e: Added braces around else

[Felix Schlepper]

This addresses two issues raised by checkpatch.pl:

     $ ./scripts/checkpatch.pl --terse -f drivers/staging/rtl8192e/rtllib_wx.c
     CHECK: braces {} should be used on all arms of this statement
     CHECK: Unbalanced braces around else statement

The coding style rule with not using unnecessary braces around if/else
does not apply if only one branch is a single statement.

Signed-off-by: Felix Schlepper <f3sch.git@outlook.com>
---
 drivers/staging/rtl8192e/rtllib_wx.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib_wx.c b/drivers/staging/rtl8192e/rtllib_wx.c
index 49b3d4c8f5d6..3db29f6eeb89 100644
--- a/drivers/staging/rtl8192e/rtllib_wx.c
+++ b/drivers/staging/rtl8192e/rtllib_wx.c
@@ -302,8 +302,9 @@ int rtllib_wx_set_encode(struct rtllib_device *ieee,
 			netdev_dbg(ieee->dev,
 				   "Disabling encryption on key %d.\n", key);
 			lib80211_crypt_delayed_deinit(&ieee->crypt_info, crypt);
-		} else
+		} else {
 			netdev_dbg(ieee->dev, "Disabling encryption.\n");
+		}
 
 		/* Check all the keys to see if any are still configured,
 		 * and if no key index was provided, de-init them all
@@ -722,8 +723,9 @@ int rtllib_wx_set_auth(struct rtllib_device *ieee,
 		} else if (data->value & IW_AUTH_ALG_LEAP) {
 			ieee->open_wep = 1;
 			ieee->auth_mode = 2;
-		} else
+		} else {
 			return -EINVAL;
+		}
 		break;
 
 	case IW_AUTH_WPA_ENABLED:
-- 
2.36.1

[PATCH v2 5/6] Staging: rtl8192e: Remove unnecessary blank line

[Felix Schlepper]

This addresses an issue raised by checkpatch.pl:

     $ ./scripts/checkpatch.pl --terse -f drivers/staging/rtl8192e/rtllib_wx.c
     CHECK: Blank lines aren't necessary before a close brace '}'

Signed-off-by: Felix Schlepper <f3sch.git@outlook.com>
---
 drivers/staging/rtl8192e/rtllib_wx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtllib_wx.c b/drivers/staging/rtl8192e/rtllib_wx.c
index 3db29f6eeb89..778b6b7e0939 100644
--- a/drivers/staging/rtl8192e/rtllib_wx.c
+++ b/drivers/staging/rtl8192e/rtllib_wx.c
@@ -599,7 +599,6 @@ int rtllib_wx_set_encode_ext(struct rtllib_device *ieee,
 			goto done;
 		}
 		*crypt = new_crypt;
-
 	}
 
 	if (ext->key_len > 0 && (*crypt)->ops->set_key &&
-- 
2.36.1

[PATCH v2 6/6] Staging: rtl8192e: Added spaces around '+'

[Felix Schlepper]

This addresses two issues raised by checkpatch.pl:

     $ ./scripts/checkpatch.pl --terse -f drivers/staging/rtl8192e/rtllib_wx.c
     CHECK: spaces preferred around that '+' (ctx:VxV)

Signed-off-by: Felix Schlepper <f3sch.git@outlook.com>
---
 drivers/staging/rtl8192e/rtllib_wx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtllib_wx.c b/drivers/staging/rtl8192e/rtllib_wx.c
index 778b6b7e0939..1bb94a1fe868 100644
--- a/drivers/staging/rtl8192e/rtllib_wx.c
+++ b/drivers/staging/rtl8192e/rtllib_wx.c
@@ -767,7 +767,7 @@ int rtllib_wx_set_gen_ie(struct rtllib_device *ieee, u8 *ie, size_t len)
 	kfree(ieee->wps_ie);
 	ieee->wps_ie = NULL;
 	if (len) {
-		if (len != ie[1]+2)
+		if (len != ie[1] + 2)
 			return -EINVAL;
 		buf = kmemdup(ie, len, GFP_KERNEL);
 		if (!buf)
-- 
2.36.1