Staging: rtl8192e: rtllib_wx.c coding style

This addresses 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.

Quoted 8 lines · Felix Schlepper

This addresses 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.

[]

drivers/staging/rtl8192e/rtllib_wx.c

Quoted 1 line

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

[]

drivers/staging/rtl8192e/rtllib_wx.c line 28

Quoted 13 lines

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

@@ -17,10 +17,12 @@
 #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},

Please always look deeper at the code and do not simply take any checkpatch suggestion as the "best" fix.

Using this struct style with an embedded length is error prone and would likely be better as a simple char * array.

Also, the existing sprintf use of mode_string and mode_size is nonsensical. There is nothing to format in any of the mode_size. so the use of mode_size as an argument is silly.

Perhaps something like this would be better:

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

drivers/staging/rtl8192e/rtllib_wx.c +3 -11

index cf9a240924f20..90dcce152d80c 100644
@@ -17,17 +17,9 @@
 #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

@@ -72,11 +64,8 @@ static inline char *rtl819x_translate_scan(struct rtllib_device *ieee,
 	/* Add the protocol name */
 	iwe.cmd = SIOCGIWNAME;

Quoted 29 lines · Joe Perches

Quoted 8 lines · Felix Schlepper

This addresses 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.

[]

drivers/staging/rtl8192e/rtllib_wx.c

Quoted 1 line

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

[]

drivers/staging/rtl8192e/rtllib_wx.c line 28

Quoted 13 lines

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

@@ -17,10 +17,12 @@
 #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},

Please always look deeper at the code and do not simply take any checkpatch suggestion as the "best" fix.

Quoted 6 lines

Using this struct style with an embedded length is error prone and would likely be better as a simple char * array.

Also, the existing sprintf use of mode_string and mode_size is nonsensical. There is nothing to format in any of the mode_size. so the use of mode_size as an argument is silly.

First of all, thank you for your suggestions.
I did realize that there is nothing to format, but I did not dare to make that change in case a format could be added.
However, looking at it now. This has not been touched in over 10 years. So it is probably safe to change that.

Quoted 30 lines

Perhaps something like this would be better:

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

drivers/staging/rtl8192e/rtllib_wx.c +3 -11

index cf9a240924f20..90dcce152d80c 100644
@@ -17,17 +17,9 @@
 #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",
 };
 

Quoted 4 lines

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

 #define MAX_CUSTOM_LEN 64

@@ -72,11 +64,8 @@ static inline char *rtl819x_translate_scan(struct rtllib_device *ieee,
 	/* Add the protocol name */
 	iwe.cmd = SIOCGIWNAME;

drivers/staging/rtl8192e/rtllib_wx.c static inline char *rtl819x_translate_scan(struct rtllib_device *ieee, · line 70

Quoted 10 lines

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

 	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))
+			pname = stpcpy(pname, rtllib_modes[i]);
 	}
 	*pname = '\0';

Would this not be also redundant since stpcpy already returns a pointer to the new NUL-terminating character?

Quoted 1 line

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

 	snprintf(iwe.u.name, IFNAMSIZ, "IEEE802.11%s", proto_name);

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

This patch does not change the logical of the assignments.

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

index 7bd1e829ff7e..c8fc66113b41 100644