public inbox for linux-kernel-mentees@lists.linux-foundation.org
 help / color / mirror / Atom feed
* [PATCH] scsi:st.c replace snprintf() with sysfs_emit()
@ 2025-07-24  0:58 Rujra Bhatt
  2025-07-24 15:24 ` Bart Van Assche
  2025-07-25 14:22 ` "Kai Mäkisara (Kolumbus)"
  0 siblings, 2 replies; 3+ messages in thread
From: Rujra Bhatt @ 2025-07-24  0:58 UTC (permalink / raw)
  To: Kai.Makisara, James.Bottomley, martin.petersen
  Cc: linux-scsi, linux-kernel, Shuah Khan, linux-kernel-mentees

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.

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);
 }
 static DRIVER_ATTR_RO(try_direct_io);

 static ssize_t fixed_buffer_size_show(struct device_driver *ddp, char *buf)
 {
-       return scnprintf(buf, PAGE_SIZE, "%d\n", st_fixed_buffer_size);
+       return sysfs_emit_at(buf, PAGE_SIZE, "%d\n", st_fixed_buffer_size);
 }
 static DRIVER_ATTR_RO(fixed_buffer_size);

 static ssize_t max_sg_segs_show(struct device_driver *ddp, char *buf)
 {
-       return scnprintf(buf, PAGE_SIZE, "%d\n", st_max_sg_segs);
+       return sysfs_emit_at(buf, PAGE_SIZE, "%d\n", st_max_sg_segs);
 }
 static DRIVER_ATTR_RO(max_sg_segs);

 static ssize_t version_show(struct device_driver *ddd, char *buf)
 {
-       return scnprintf(buf, PAGE_SIZE, "[%s]\n", verstr);
+       return sysfs_emit_at(buf, PAGE_SIZE, "[%s]\n", verstr);
 }
 static DRIVER_ATTR_RO(version);

@@ -4608,7 +4608,7 @@ static ssize_t debug_flag_store(struct device_driver *ddp,

 static ssize_t debug_flag_show(struct device_driver *ddp, char *buf)
 {
-       return scnprintf(buf, PAGE_SIZE, "%d\n", debugging);
+       return sysfs_emit_at(buf, PAGE_SIZE, "%d\n", debugging);
 }
 static DRIVER_ATTR_RW(debug_flag);
 #endif
@@ -4632,7 +4632,7 @@ defined_show(struct device *dev, struct
device_attribute *attr, char *buf)
        struct st_modedef *STm = dev_get_drvdata(dev);
        ssize_t l = 0;

-       l = snprintf(buf, PAGE_SIZE, "%d\n", STm->defined);
+       l = sysfs_emit_at(buf, PAGE_SIZE, "%d\n", STm->defined);
        return l;
 }
 static DEVICE_ATTR_RO(defined);
@@ -4644,7 +4644,7 @@ default_blksize_show(struct device *dev, struct
device_attribute *attr,
        struct st_modedef *STm = dev_get_drvdata(dev);
        ssize_t l = 0;

-       l = snprintf(buf, PAGE_SIZE, "%d\n", STm->default_blksize);
+       l = sysfs_emit_at(buf, PAGE_SIZE, "%d\n", STm->default_blksize);
        return l;
 }
 static DEVICE_ATTR_RO(default_blksize);
@@ -4658,7 +4658,7 @@ default_density_show(struct device *dev, struct
device_attribute *attr,
        char *fmt;

        fmt = STm->default_density >= 0 ? "0x%02x\n" : "%d\n";
-       l = snprintf(buf, PAGE_SIZE, fmt, STm->default_density);
+       l = sysfs_emit_at(buf, PAGE_SIZE, fmt, STm->default_density);
        return l;
 }
 static DEVICE_ATTR_RO(default_density);
@@ -4670,7 +4670,7 @@ default_compression_show(struct device *dev,
struct device_attribute *attr,
        struct st_modedef *STm = dev_get_drvdata(dev);
        ssize_t l = 0;

-       l = snprintf(buf, PAGE_SIZE, "%d\n", STm->default_compression - 1);
+       l = sysfs_emit_at(buf, PAGE_SIZE, "%d\n", STm->default_compression - 1);
        return l;
 }
 static DEVICE_ATTR_RO(default_compression);
@@ -4699,7 +4699,7 @@ options_show(struct device *dev, struct
device_attribute *attr, char *buf)
        options |= STp->immediate_filemark ? MT_ST_NOWAIT_EOF : 0;
        options |= STp->sili ? MT_ST_SILI : 0;

-       l = snprintf(buf, PAGE_SIZE, "0x%08x\n", options);
+       l = sysfs_emit_at(buf, PAGE_SIZE, "0x%08x\n", options);
        return l;
 }
 static DEVICE_ATTR_RO(options);
@@ -4718,7 +4718,7 @@ static ssize_t
position_lost_in_reset_show(struct device *dev,
        struct st_modedef *STm = dev_get_drvdata(dev);
        struct scsi_tape *STp = STm->tape;

-       return sprintf(buf, "%d", STp->pos_unknown);
+       return sysfs_emit(buf, "%d", STp->pos_unknown);
 }
 static DEVICE_ATTR_RO(position_lost_in_reset);

@@ -4735,7 +4735,7 @@ static ssize_t read_cnt_show(struct device *dev,
 {
        struct st_modedef *STm = dev_get_drvdata(dev);

-       return sprintf(buf, "%lld",
+       return sysfs_emit(buf, "%lld",
                       (long long)atomic64_read(&STm->tape->stats->read_cnt));
 }
 static DEVICE_ATTR_RO(read_cnt);
@@ -4753,7 +4753,7 @@ static ssize_t read_byte_cnt_show(struct device *dev,
 {
        struct st_modedef *STm = dev_get_drvdata(dev);

-       return sprintf(buf, "%lld",
+       return sysfs_emit(buf, "%lld",
                       (long
long)atomic64_read(&STm->tape->stats->read_byte_cnt));
 }
 static DEVICE_ATTR_RO(read_byte_cnt);
@@ -4769,7 +4769,7 @@ static ssize_t read_ns_show(struct device *dev,
 {
        struct st_modedef *STm = dev_get_drvdata(dev);

-       return sprintf(buf, "%lld",
+       return sysfs_emit(buf, "%lld",
                       (long
long)atomic64_read(&STm->tape->stats->tot_read_time));
 }
 static DEVICE_ATTR_RO(read_ns);
@@ -4786,7 +4786,7 @@ static ssize_t write_cnt_show(struct device *dev,
 {
        struct st_modedef *STm = dev_get_drvdata(dev);

-       return sprintf(buf, "%lld",
+       return sysfs_emit(buf, "%lld",
                       (long long)atomic64_read(&STm->tape->stats->write_cnt));
 }
 static DEVICE_ATTR_RO(write_cnt);
@@ -4803,7 +4803,7 @@ static ssize_t write_byte_cnt_show(struct device *dev,
 {
        struct st_modedef *STm = dev_get_drvdata(dev);

-       return sprintf(buf, "%lld",
+       return sysfs_emit(buf, "%lld",
                       (long
long)atomic64_read(&STm->tape->stats->write_byte_cnt));
 }
 static DEVICE_ATTR_RO(write_byte_cnt);
@@ -4820,7 +4820,7 @@ static ssize_t write_ns_show(struct device *dev,
 {
        struct st_modedef *STm = dev_get_drvdata(dev);

-       return sprintf(buf, "%lld",
+       return sysfs_emit(buf, "%lld",
                       (long
long)atomic64_read(&STm->tape->stats->tot_write_time));
 }
 static DEVICE_ATTR_RO(write_ns);
@@ -4838,7 +4838,7 @@ static ssize_t in_flight_show(struct device *dev,
 {
        struct st_modedef *STm = dev_get_drvdata(dev);

-       return sprintf(buf, "%lld",
+       return sysfs_emit(buf, "%lld",
                       (long long)atomic64_read(&STm->tape->stats->in_flight));
 }
 static DEVICE_ATTR_RO(in_flight);
@@ -4858,7 +4858,7 @@ static ssize_t io_ns_show(struct device *dev,
 {
        struct st_modedef *STm = dev_get_drvdata(dev);

-       return sprintf(buf, "%lld",
+       return sysfs_emit(buf, "%lld",
                       (long
long)atomic64_read(&STm->tape->stats->tot_io_time));
 }
 static DEVICE_ATTR_RO(io_ns);
@@ -4877,7 +4877,7 @@ static ssize_t other_cnt_show(struct device *dev,
 {
        struct st_modedef *STm = dev_get_drvdata(dev);

-       return sprintf(buf, "%lld",
+       return sysfs_emit(buf, "%lld",
                       (long long)atomic64_read(&STm->tape->stats->other_cnt));
 }
 static DEVICE_ATTR_RO(other_cnt);
@@ -4895,7 +4895,7 @@ static ssize_t resid_cnt_show(struct device *dev,
 {
        struct st_modedef *STm = dev_get_drvdata(dev);

-       return sprintf(buf, "%lld",
+       return sysfs_emit(buf, "%lld",
                       (long long)atomic64_read(&STm->tape->stats->resid_cnt));
 }
 static DEVICE_ATTR_RO(resid_cnt);
--
2.43.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] scsi:st.c replace snprintf() with sysfs_emit()
  2025-07-24  0:58 [PATCH] scsi:st.c replace snprintf() with sysfs_emit() Rujra Bhatt
@ 2025-07-24 15:24 ` Bart Van Assche
  2025-07-25 14:22 ` "Kai Mäkisara (Kolumbus)"
  1 sibling, 0 replies; 3+ messages in thread
From: Bart Van Assche @ 2025-07-24 15:24 UTC (permalink / raw)
  To: Rujra Bhatt, Kai.Makisara, James.Bottomley, martin.petersen
  Cc: linux-scsi, linux-kernel, Shuah Khan, linux-kernel-mentees

On 7/23/25 5:58 PM, Rujra Bhatt 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.

sysfs_emit() is for new code only. There is no requirement to convert
existing code to sysfs_emit() / sysfs_emit_at().

Bart.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] scsi:st.c replace snprintf() with sysfs_emit()
  2025-07-24  0:58 [PATCH] scsi:st.c replace snprintf() with sysfs_emit() Rujra Bhatt
  2025-07-24 15:24 ` Bart Van Assche
@ 2025-07-25 14:22 ` "Kai Mäkisara (Kolumbus)"
  1 sibling, 0 replies; 3+ messages in thread
From: "Kai Mäkisara (Kolumbus)" @ 2025-07-25 14:22 UTC (permalink / raw)
  To: Rujra Bhatt
  Cc: James.Bottomley, martin.petersen, linux-scsi, linux-kernel,
	Shuah Khan, linux-kernel-mentees, bvanassche


> 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-25 14:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-24  0:58 [PATCH] scsi:st.c replace snprintf() with sysfs_emit() Rujra Bhatt
2025-07-24 15:24 ` Bart Van Assche
2025-07-25 14:22 ` "Kai Mäkisara (Kolumbus)"

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox