lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f110442f5cceb0161f32c2b942ef7be6f5c5a63c.camel@perches.com>
Date:   Tue, 28 Jun 2022 10:58:23 -0700
From:   Joe Perches <joe@...ches.com>
To:     Felix Schlepper <f3sch.git@...look.com>, gregkh@...uxfoundation.org
Cc:     linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        wjsota@...il.com
Subject: Re: [PATCH 1/7] Staging: rtl8192e: Added blank lines before/after
 struct

On Tue, 2022-06-28 at 10:30 +0200, Felix Schlepper wrote:
> 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.
[]
> diff --git a/drivers/staging/rtl8192e/rtllib_wx.c b/drivers/staging/rtl8192e/rtllib_wx.c
[]
> @@ -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(-)

diff --git a/drivers/staging/rtl8192e/rtllib_wx.c b/drivers/staging/rtl8192e/rtllib_wx.c
index cf9a240924f20..90dcce152d80c 100644
--- a/drivers/staging/rtl8192e/rtllib_wx.c
+++ b/drivers/staging/rtl8192e/rtllib_wx.c
@@ -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;
 	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';
 	snprintf(iwe.u.name, IFNAMSIZ, "IEEE802.11%s", proto_name);

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ