[PATCH linux-next v2] net: stmmac: use sysfs_streq() instead of strncmp()

[PATCH linux-next v2] net: stmmac: use sysfs_streq() instead of strncmp()

[yang.yang29]

From: Xu Panda <xu.panda@zte.com.cn>

Replace the open-code with sysfs_streq().

---
change for v2
 - fix the mistake of redundant parameter.
---
Signed-off-by: Xu Panda <xu.panda@zte.com.cn>
Signed-off-by: Yang Yang <yang.yang29@zte.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 0a9d13d7976f..5ec9f64dadd0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7562,31 +7562,31 @@ static int __init stmmac_cmdline_opt(char *str)
 	if (!str || !*str)
 		return 1;
 	while ((opt = strsep(&str, ",")) != NULL) {
-		if (!strncmp(opt, "debug:", 6)) {
+		if (sysfs_streq(opt, "debug:")) {
 			if (kstrtoint(opt + 6, 0, &debug))
 				goto err;
-		} else if (!strncmp(opt, "phyaddr:", 8)) {
+		} else if (sysfs_streq(opt, "phyaddr:")) {
 			if (kstrtoint(opt + 8, 0, &phyaddr))
 				goto err;
-		} else if (!strncmp(opt, "buf_sz:", 7)) {
+		} else if (sysfs_streq(opt, "buf_sz:")) {
 			if (kstrtoint(opt + 7, 0, &buf_sz))
 				goto err;
-		} else if (!strncmp(opt, "tc:", 3)) {
+		} else if (sysfs_streq(opt, "tc:")) {
 			if (kstrtoint(opt + 3, 0, &tc))
 				goto err;
-		} else if (!strncmp(opt, "watchdog:", 9)) {
+		} else if (sysfs_streq(opt, "watchdog:")) {
 			if (kstrtoint(opt + 9, 0, &watchdog))
 				goto err;
-		} else if (!strncmp(opt, "flow_ctrl:", 10)) {
+		} else if (sysfs_streq(opt, "flow_ctrl:")) {
 			if (kstrtoint(opt + 10, 0, &flow_ctrl))
 				goto err;
-		} else if (!strncmp(opt, "pause:", 6)) {
+		} else if (sysfs_streq(opt, "pause:")) {
 			if (kstrtoint(opt + 6, 0, &pause))
 				goto err;
-		} else if (!strncmp(opt, "eee_timer:", 10)) {
+		} else if (sysfs_streq(opt, "eee_timer:")) {
 			if (kstrtoint(opt + 10, 0, &eee_timer))
 				goto err;
-		} else if (!strncmp(opt, "chain_mode:", 11)) {
+		} else if (sysfs_streq(opt, "chain_mode:")) {
 			if (kstrtoint(opt + 11, 0, &chain_mode))
 				goto err;
 		}
-- 
2.15.2

Re: [PATCH linux-next v2] net: stmmac: use sysfs_streq() instead of strncmp()

[Andrew Lunn]

On Tue, Nov 29, 2022 at 03:02:28PM +0800, yang.yang29@zte.com.cn wrote:
> From: Xu Panda <xu.panda@zte.com.cn>
> 
> Replace the open-code with sysfs_streq().
> 
> ---
> change for v2
>  - fix the mistake of redundant parameter.

> -		} else if (!strncmp(opt, "tc:", 3)) {
> +		} else if (sysfs_streq(opt, "tc:")) {
>  			if (kstrtoint(opt + 3, 0, &tc))
>  				goto err;

Vladimir made the comment:

> What's even worse is that the patch is flat out wrong. The stmmac_cmdline_opt()
> function does not parse sysfs input, but cmdline input such as
> "stmmaceth=tc:1,pause:1". The pattern of using strsep() followed by
> strncmp() for such strings is not unique to stmmac, it can also be found
> mainly in drivers under drivers/video/fbdev/.
> 
> With strncmp("tc:", 3), the code matches on the "tc:1" token properly.
> With sysfs_streq("tc:"), it doesn't.

It is not clear you have addressed this point.

   Andrew

Re: [PATCH linux-next v2] net: stmmac: use sysfs_streq() instead of strncmp()

[Vladimir Oltean]

On Tue, Nov 29, 2022 at 07:25:41PM +0100, Andrew Lunn wrote:
> > With strncmp("tc:", 3), the code matches on the "tc:1" token properly.
> > With sysfs_streq("tc:"), it doesn't.
> 
> It is not clear you have addressed this point.

As they say, "let sleeping dogs cry". I'm not sure that we should be
making (especially untested) changes in the cmdline handling there.