[PATCH] tty: vt: replace snprintf in show functions with sysfs_emit

[PATCH] tty: vt: replace snprintf in show functions with sysfs_emit

[Qing Wang]

show() must not use snprintf() when formatting the value to be
returned to user space.

Fix the following coccicheck warning:
drivers/tty/vt/vt.c:3902: WARNING: use scnprintf or sprintf.
drivers/tty/vt/vt.c:3910: WARNING: use scnprintf or sprintf.

Signed-off-by: Qing Wang <wangqing@vivo.com>
---
 drivers/tty/vt/vt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index ef981d3..ea44ea1 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3899,7 +3899,7 @@ static ssize_t show_bind(struct device *dev, struct device_attribute *attr,
 	bind = con_is_bound(con->con);
 	console_unlock();
 
-	return snprintf(buf, PAGE_SIZE, "%i\n", bind);
+	return sysfs_emit(buf, "%i\n", bind);
 }
 
 static ssize_t show_name(struct device *dev, struct device_attribute *attr,
@@ -3907,7 +3907,7 @@ static ssize_t show_name(struct device *dev, struct device_attribute *attr,
 {
 	struct con_driver *con = dev_get_drvdata(dev);
 
-	return snprintf(buf, PAGE_SIZE, "%s %s\n",
+	return sysfs_emit(buf, "%s %s\n",
 			(con->flag & CON_DRIVER_FLAG_MODULE) ? "(M)" : "(S)",
 			 con->desc);
 
-- 
2.7.4

Re: [PATCH] tty: vt: replace snprintf in show functions with sysfs_emit

[Greg Kroah-Hartman]

On Thu, Oct 14, 2021 at 11:51:36PM -0700, Qing Wang wrote:
> show() must not use snprintf() when formatting the value to be
> returned to user space.

Again, who is making this "must" requirement?

I, as the sysfs maintainer, am not saying that all existing show
functions MUST be converted, so I find it hard to believe that someone
else is...


> 
> Fix the following coccicheck warning:
> drivers/tty/vt/vt.c:3902: WARNING: use scnprintf or sprintf.
> drivers/tty/vt/vt.c:3910: WARNING: use scnprintf or sprintf.

Someone needs to change this warning to show the correct thing here.

thanks,

greg k-h

回复: [PATCH] tty: vt: replace snprintf in show functions with sysfs_emit

[王擎]

>> show() must not use snprintf() when formatting the value to be
>> returned to user space.
>
>Again, who is making this "must" requirement?
>
>I, as the sysfs maintainer, am not saying that all existing show
>functions MUST be converted, so I find it hard to believe that someone
>else is...
>

According to Documentation/filesystems/sysfs.txt:
show() methods of device attributes should return the number
of bytes printed into the buffer. This is the return value of 
scnprintf(). snprintf() returns the length the resulting string.

So, show() must not use snprintf() when formatting 
the value to be returned to user space. 
Also, use sysfs_emit directly makes more sense.

Thanks,

Qing

>
>> 
>> Fix the following coccicheck warning:
>> drivers/tty/vt/vt.c:3902: WARNING: use scnprintf or sprintf.
>> drivers/tty/vt/vt.c:3910: WARNING: use scnprintf or sprintf.
>
>Someone needs to change this warning to show the correct thing here.
>
>thanks,
>
>greg k-h

Re: 回复: [PATCH] tty: vt: replace snprintf in show functions with sysfs_emit

[Greg Kroah-Hartman]

On Fri, Oct 15, 2021 at 07:10:42AM +0000, 王擎 wrote:
> 
> >> show() must not use snprintf() when formatting the value to be
> >> returned to user space.
> >
> >Again, who is making this "must" requirement?
> >
> >I, as the sysfs maintainer, am not saying that all existing show
> >functions MUST be converted, so I find it hard to believe that someone
> >else is...
> >
> 
> According to Documentation/filesystems/sysfs.txt:
> show() methods of device attributes should return the number
> of bytes printed into the buffer. This is the return value of 
> scnprintf(). snprintf() returns the length the resulting string.
> 
> So, show() must not use snprintf() when formatting 
> the value to be returned to user space. 

Ok, then you need to document this _MUCH_ better, saying that the value
returned by the kernel today is WRONG, and that this is a bugfix.

And can you see the difference here?  Have you tested these?

> Also, use sysfs_emit directly makes more sense.

I agree, but your changelog did not mention that at all.

thanks,

greg k-h