* [PATCH] USB: usbsevseg: cleanup sysfs show handlers
@ 2026-10-29 12:47 Dmitry Antipov
2026-10-29 12:53 ` sashiko-bot
0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Antipov @ 2026-10-29 12:47 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Johan Hovold, linux-usb, Dmitry Antipov
Prefer the convenient sysfs_emit() and sysfs_emit_at() over
sprintf() and strcat()-based quirks in sysfs show handlers.
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
drivers/usb/misc/usbsevseg.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/misc/usbsevseg.c b/drivers/usb/misc/usbsevseg.c
index 89d25fcef642..f456a7161a26 100644
--- a/drivers/usb/misc/usbsevseg.c
+++ b/drivers/usb/misc/usbsevseg.c
@@ -145,7 +145,7 @@ static ssize_t name##_show(struct device *dev, \
struct usb_interface *intf = to_usb_interface(dev); \
struct usb_sevsegdev *mydev = usb_get_intfdata(intf); \
\
- return sprintf(buf, "%u\n", mydev->name); \
+ return sysfs_emit(buf, "%u\n", mydev->name); \
} \
\
static ssize_t name##_store(struct device *dev, \
@@ -246,25 +246,19 @@ static ssize_t textmode_show(struct device *dev,
{
struct usb_interface *intf = to_usb_interface(dev);
struct usb_sevsegdev *mydev = usb_get_intfdata(intf);
- int i;
-
- buf[0] = 0;
+ int i, ret = 0;
for (i = 0; i < ARRAY_SIZE(display_textmodes); i++) {
- if (mydev->textmode == i) {
- strcat(buf, " [");
- strcat(buf, display_textmodes[i]);
- strcat(buf, "] ");
- } else {
- strcat(buf, " ");
- strcat(buf, display_textmodes[i]);
- strcat(buf, " ");
- }
+ if (mydev->textmode == i)
+ ret += sysfs_emit_at(buf, ret, " [%s] ",
+ display_textmodes[i]);
+ else
+ ret += sysfs_emit_at(buf, ret, " %s ",
+ display_textmodes[i]);
}
- strcat(buf, "\n");
-
- return strlen(buf);
+ ret += sysfs_emit_at(buf, ret - 1, "\n");
+ return ret;
}
static ssize_t textmode_store(struct device *dev,
--
2.56.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] USB: usbsevseg: cleanup sysfs show handlers
2026-10-29 12:47 [PATCH] USB: usbsevseg: cleanup sysfs show handlers Dmitry Antipov
@ 2026-10-29 12:53 ` sashiko-bot
2026-10-29 13:14 ` [PATCH v2] " Dmitry Antipov
0 siblings, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-10-29 12:53 UTC (permalink / raw)
To: Dmitry Antipov; +Cc: linux-usb
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] usb: usbsevseg: off-by-one in textmode_show() emits trailing NUL byte
--
commit a74c1b2c71cf0a70caf0b902cff95d63b0e182d0
Author: Dmitry Antipov <dmantipov@yandex.ru>
USB: usbsevseg: cleanup sysfs show handlers
This commit replaces sprintf() and strcat() calls in usbsevseg sysfs show
handlers with sysfs_emit() and sysfs_emit_at().
> diff --git a/drivers/usb/misc/usbsevseg.c b/drivers/usb/misc/usbsevseg.c
> index 89d25fcef6424..f456a7161a260 100644
> --- a/drivers/usb/misc/usbsevseg.c
> +++ b/drivers/usb/misc/usbsevseg.c
[ ... ]
> @@ -246,25 +246,19 @@ static ssize_t textmode_show(struct device *dev,
> {
> struct usb_interface *intf = to_usb_interface(dev);
> struct usb_sevsegdev *mydev = usb_get_intfdata(intf);
> - int i;
> -
> - buf[0] = 0;
> + int i, ret = 0;
>
> for (i = 0; i < ARRAY_SIZE(display_textmodes); i++) {
> - if (mydev->textmode == i) {
> - strcat(buf, " [");
> - strcat(buf, display_textmodes[i]);
> - strcat(buf, "] ");
> - } else {
> - strcat(buf, " ");
> - strcat(buf, display_textmodes[i]);
> - strcat(buf, " ");
> - }
> + if (mydev->textmode == i)
> + ret += sysfs_emit_at(buf, ret, " [%s] ",
> + display_textmodes[i]);
> + else
> + ret += sysfs_emit_at(buf, ret, " %s ",
> + display_textmodes[i]);
> }
> - strcat(buf, "\n");
> -
>
> - return strlen(buf);
> + ret += sysfs_emit_at(buf, ret - 1, "\n");
> + return ret;
[Severity: Medium]
Does passing ret - 1 to sysfs_emit_at() while still adding its return value
to ret cause textmode_show() in drivers/usb/misc/usbsevseg.c to include a
trailing NUL byte in the output returned to userspace when reading the
textmode sysfs attribute?
At the end of the loop, buf[0 .. ret - 1] holds the formatted string with a
trailing space at buf[ret - 1] and a NUL terminator at buf[ret]. Calling
sysfs_emit_at(buf, ret - 1, "\n") overwrites the space at buf[ret - 1] with
a newline, writes the NUL terminator at buf[ret], and returns 1.
Adding that return value to ret increments ret to strlen(buf) + 1. When
sysfs_kf_seq_show() in fs/sysfs/file.c calls seq_commit(sf, count) with that
returned count, the NUL byte at buf[ret - 1] is committed into the seq_file
buffer and exposed to userspace.
Should this either pass ret instead of ret - 1 to preserve the original
trailing space before the newline, or avoid incrementing ret if overwriting
the trailing space was intentional?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029124716.611540-1-dmantipov@yandex.ru?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2] USB: usbsevseg: cleanup sysfs show handlers
2026-10-29 12:53 ` sashiko-bot
@ 2026-10-29 13:14 ` Dmitry Antipov
2026-10-29 13:17 ` sashiko-bot
0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Antipov @ 2026-10-29 13:14 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Johan Hovold, linux-usb, Dmitry Antipov
Prefer the convenient sysfs_emit() and sysfs_emit_at() over
sprintf() and strcat()-based quirks in sysfs show handlers.
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: fix off-by-one error in textmode_show() (Sashiko)
---
drivers/usb/misc/usbsevseg.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/misc/usbsevseg.c b/drivers/usb/misc/usbsevseg.c
index 89d25fcef642..1140ca4fbd8d 100644
--- a/drivers/usb/misc/usbsevseg.c
+++ b/drivers/usb/misc/usbsevseg.c
@@ -145,7 +145,7 @@ static ssize_t name##_show(struct device *dev, \
struct usb_interface *intf = to_usb_interface(dev); \
struct usb_sevsegdev *mydev = usb_get_intfdata(intf); \
\
- return sprintf(buf, "%u\n", mydev->name); \
+ return sysfs_emit(buf, "%u\n", mydev->name); \
} \
\
static ssize_t name##_store(struct device *dev, \
@@ -246,25 +246,19 @@ static ssize_t textmode_show(struct device *dev,
{
struct usb_interface *intf = to_usb_interface(dev);
struct usb_sevsegdev *mydev = usb_get_intfdata(intf);
- int i;
-
- buf[0] = 0;
+ int i, ret = 0;
for (i = 0; i < ARRAY_SIZE(display_textmodes); i++) {
- if (mydev->textmode == i) {
- strcat(buf, " [");
- strcat(buf, display_textmodes[i]);
- strcat(buf, "] ");
- } else {
- strcat(buf, " ");
- strcat(buf, display_textmodes[i]);
- strcat(buf, " ");
- }
+ if (mydev->textmode == i)
+ ret += sysfs_emit_at(buf, ret, " [%s] ",
+ display_textmodes[i]);
+ else
+ ret += sysfs_emit_at(buf, ret, " %s ",
+ display_textmodes[i]);
}
- strcat(buf, "\n");
-
- return strlen(buf);
+ sysfs_emit_at(buf, ret - 1, "\n");
+ return ret;
}
static ssize_t textmode_store(struct device *dev,
--
2.56.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-29 13:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29 12:47 [PATCH] USB: usbsevseg: cleanup sysfs show handlers Dmitry Antipov
2026-10-29 12:53 ` sashiko-bot
2026-10-29 13:14 ` [PATCH v2] " Dmitry Antipov
2026-10-29 13:17 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox