Staging: rtl8192e: rtllib_wx codingstyle/refactor

PATCH Superseded Felix Schlepper 2 participants 8 messages Jun 30, 2022 – Jul 1, 2022 Staging

lore

Share & tools

lore

Felix Schlepper

author

2022-06-30 11:30 UTC #0 lore raw

v2 cover Staging: rtl8192e: rtllib_wx codingstyle/refactor

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

Signature · 1 line

2.36.1

Felix Schlepper

author

2022-06-30 11:30 UTC #1 lore raw

PATCH v2 1/6 Staging: rtl8192e: Refactored rtllib_modes

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

---
 drivers/staging/rtl8192e/rtllib_wx.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

drivers/staging/rtl8192e/rtllib_wx.c +5 ~~-16~~

index cf9a240924f2..1e420230d029 100644
@@ -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';

Signature · 1 line

2.36.1

Felix Schlepper

author

2022-06-30 11:30 UTC #5 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/rtl8192e/rtllib_wx.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/staging/rtl8192e/rtllib_wx.c +4 -2

index 49b3d4c8f5d6..3db29f6eeb89 100644
@@ -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:

Signature · 1 line

2.36.1

Felix Schlepper

author

2022-06-30 11:30 UTC #7 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/rtl8192e/rtllib_wx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/rtl8192e/rtllib_wx.c +1 -1

index 778b6b7e0939..1bb94a1fe868 100644
@@ -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)

Signature · 1 line

2.36.1

Felix Schlepper

author

2022-06-30 11:30 UTC #3 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/rtl8192e/rtllib_wx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/staging/rtl8192e/rtllib_wx.c +2 -1

index 1e420230d029..f8683b914668 100644
@@ -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;

Signature · 1 line

2.36.1

Felix Schlepper

author

2022-06-30 11:30 UTC #4 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/rtl8192e/rtllib_wx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/staging/rtl8192e/rtllib_wx.c +2 -2

index f8683b914668..49b3d4c8f5d6 100644
@@ -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;

Signature · 1 line

2.36.1

Felix Schlepper

author

2022-06-30 11:30 UTC #6 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/rtl8192e/rtllib_wx.c | 1 -
 1 file changed, 1 deletion(-)

drivers/staging/rtl8192e/rtllib_wx.c +0 -1

index 3db29f6eeb89..778b6b7e0939 100644
@@ -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 &&

Signature · 1 line

2.36.1

Greg KH

maintainer

2022-07-01 07:43 UTC #2 lore raw

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

In reply to Felix Schlepper

Quoted 62 lines · 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 <[email protected]>

---
 drivers/staging/rtl8192e/rtllib_wx.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

drivers/staging/rtl8192e/rtllib_wx.c +5 ~~-16~~

index cf9a240924f2..1e420230d029 100644
@@ -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';

Signature · 1 line

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