Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

Revert "net: stmmac: use sysfs_streq() instead of strncmp()"

This reverts commit f72cd76b05ea1ce9258484e8127932d0ea928f22.
This patch is so broken, it hurts. Apparently no one reviewed it and it
passed the build testing (because the code was compiled out), but it was
obviously never compile-tested, since it produces the following build
error, due to an incomplete conversion where an extra argument was left,
although the function being called was left:

stmmac_main.c: In function ‘stmmac_cmdline_opt’:
stmmac_main.c:7586:28: error: too many arguments to function ‘sysfs_streq’
7586 | } else if (sysfs_streq(opt, "pause:", 6)) {
| ^~~~~~~~~~~
In file included from ../include/linux/bitmap.h:11,
from ../include/linux/cpumask.h:12,
from ../include/linux/smp.h:13,
from ../include/linux/lockdep.h:14,
from ../include/linux/mutex.h:17,
from ../include/linux/notifier.h:14,
from ../include/linux/clk.h:14,
from ../drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:17:
../include/linux/string.h:185:13: note: declared here
185 | extern bool sysfs_streq(const char *s1, const char *s2);
| ^~~~~~~~~~~

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.

Fixes: f72cd76b05ea ("net: stmmac: use sysfs_streq() instead of strncmp()")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Link: https://lore.kernel.org/r/20221125105304.3012153-1-vladimir.oltean@nxp.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Vladimir Oltean and committed by
Jakub Kicinski
469d258d 2816c986

+9 -9
+9 -9
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
··· 7562 7562 if (!str || !*str) 7563 7563 return 1; 7564 7564 while ((opt = strsep(&str, ",")) != NULL) { 7565 - if (sysfs_streq(opt, "debug:")) { 7565 + if (!strncmp(opt, "debug:", 6)) { 7566 7566 if (kstrtoint(opt + 6, 0, &debug)) 7567 7567 goto err; 7568 - } else if (sysfs_streq(opt, "phyaddr:")) { 7568 + } else if (!strncmp(opt, "phyaddr:", 8)) { 7569 7569 if (kstrtoint(opt + 8, 0, &phyaddr)) 7570 7570 goto err; 7571 - } else if (sysfs_streq(opt, "buf_sz:")) { 7571 + } else if (!strncmp(opt, "buf_sz:", 7)) { 7572 7572 if (kstrtoint(opt + 7, 0, &buf_sz)) 7573 7573 goto err; 7574 - } else if (sysfs_streq(opt, "tc:")) { 7574 + } else if (!strncmp(opt, "tc:", 3)) { 7575 7575 if (kstrtoint(opt + 3, 0, &tc)) 7576 7576 goto err; 7577 - } else if (sysfs_streq(opt, "watchdog:")) { 7577 + } else if (!strncmp(opt, "watchdog:", 9)) { 7578 7578 if (kstrtoint(opt + 9, 0, &watchdog)) 7579 7579 goto err; 7580 - } else if (sysfs_streq(opt, "flow_ctrl:")) { 7580 + } else if (!strncmp(opt, "flow_ctrl:", 10)) { 7581 7581 if (kstrtoint(opt + 10, 0, &flow_ctrl)) 7582 7582 goto err; 7583 - } else if (sysfs_streq(opt, "pause:", 6)) { 7583 + } else if (!strncmp(opt, "pause:", 6)) { 7584 7584 if (kstrtoint(opt + 6, 0, &pause)) 7585 7585 goto err; 7586 - } else if (sysfs_streq(opt, "eee_timer:")) { 7586 + } else if (!strncmp(opt, "eee_timer:", 10)) { 7587 7587 if (kstrtoint(opt + 10, 0, &eee_timer)) 7588 7588 goto err; 7589 - } else if (sysfs_streq(opt, "chain_mode:")) { 7589 + } else if (!strncmp(opt, "chain_mode:", 11)) { 7590 7590 if (kstrtoint(opt + 11, 0, &chain_mode)) 7591 7591 goto err; 7592 7592 }