2022-11-29 07:32:48

by Yang Yang

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

From: Xu Panda <[email protected]>

Replace the open-code with sysfs_streq().

---
change for v2
- fix the mistake of redundant parameter.
---
Signed-off-by: Xu Panda <[email protected]>
Signed-off-by: Yang Yang <[email protected]>
---
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

2022-11-29 18:56:27

by Andrew Lunn

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

On Tue, Nov 29, 2022 at 03:02:28PM +0800, [email protected] wrote:
> From: Xu Panda <[email protected]>
>
> 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

2022-11-29 19:13:47

by Vladimir Oltean

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

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.