Re: [PATCH] scsi:st.c replace snprintf() with sysfs_emit()

"Kai Mäkisara (Kolumbus)"
2025-07-25T14:23:23Z

> On 24. Jul 2025, at 3.58, Rujra Bhatt <braker.noob.kernel@gmail.com> wrote:
>
> replace snprintf() with sysfs_emit() or sysfs_emit_at() in st.c file to
> follow kernel's guidelines from Documentation/filesystems/sysfs.rst
> This improves safety, consistency and easier to maintain and update it
> in the future.
>
Bart already wrote that these changes are required for existing drivers. I am a little hesitant about these kind of changes that don't fix anything and don't add improvements. Any changes risk breaking something.

But, since I have wondered what these sysfs_emit* functions do, I decided to take a look...

> Signed-off-by: Rujra Bhatt <braker.noob.kernel@gmail.com>
> ---
> drivers/scsi/st.c | 42 +++++++++++++++++++++---------------------
> 1 file changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index 74a6830b7ed8..38badba472d7 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -4564,25 +4564,25 @@ module_exit(exit_st);
> /* The sysfs driver interface. Read-only at the moment */
> static ssize_t try_direct_io_show(struct device_driver *ddp, char *buf)
> {
> - return scnprintf(buf, PAGE_SIZE, "%d\n", try_direct_io);
> + return sysfs_emit_at(buf, PAGE_SIZE, "%d\n", try_direct_io);

This "maps back" to vscnprintf(buf + PAGE_SIZE, 0, "%d\n", try_direct_io), which does not match the existing code (i.e., would return nothing). (Actually the problem is caught already by the check at the beginning of sysfs_emit_at()).

I am not happy to find out that this patch seems not to have been tested before submission. (Well, maybe compile tested.)

Kai