| Message ID | Zw2GRQkbx8Z8DlcS@mail.google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [next] mips: sgi-ip22: Replace "s[n]?printf" with sysfs_emit in sysfs callbacks | expand |
On Tue, 15 Oct 2024, Paulo Miguel Almeida wrote: > snprintf() has the documented, but still rather strange trait of > returning the length of the data that *would have been* written to the > array if space were available, rather than the arguably more useful > length of data *actually* written, [...] Why do you think that just returning `n - 1' in the case of a length overflow would be more useful than returning the unmet buffer length requirement? I think the opposite is the case: the value returned lets you reallocate the buffer for more space and retry, and there's no other way to figure out how much this would be. And if you need to know how many characters were actually written, then `min(n - 1, snprintf(...))' will do (and code you propose to replace does exactly that, open-coded). The change itself makes sense to me, but not your proposed description I'm afraid. Just replacing open-coded pieces with calls to `sysfs_emit' is enough justification. Maciej
On Tue, Oct 15, 2024 at 03:01:13AM +0100, Maciej W. Rozycki wrote: > On Tue, 15 Oct 2024, Paulo Miguel Almeida wrote: > > > snprintf() has the documented, but still rather strange trait of > > returning the length of the data that *would have been* written to the > > array if space were available, rather than the arguably more useful > > length of data *actually* written, [...] > > Why do you think that just returning `n - 1' in the case of a length > overflow would be more useful than returning the unmet buffer length > requirement? I think the opposite is the case: the value returned lets > you reallocate the buffer for more space and retry, and there's no other > way to figure out how much this would be. And if you need to know how > many characters were actually written, then `min(n - 1, snprintf(...))' > will do (and code you propose to replace does exactly that, open-coded). > > The change itself makes sense to me, but not your proposed description > I'm afraid. Just replacing open-coded pieces with calls to `sysfs_emit' > is enough justification. > > Maciej Thanks for taking the time to review this patch. Will submit a v2 with the description you pointed out. - Paulo A.
diff --git a/arch/mips/sgi-ip22/ip22-gio.c b/arch/mips/sgi-ip22/ip22-gio.c index d20eec742bfa..5893ea4e382c 100644 --- a/arch/mips/sgi-ip22/ip22-gio.c +++ b/arch/mips/sgi-ip22/ip22-gio.c @@ -165,9 +165,8 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a, char *buf) { struct gio_device *gio_dev = to_gio_device(dev); - int len = snprintf(buf, PAGE_SIZE, "gio:%x\n", gio_dev->id.id); - return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len; + return sysfs_emit(buf, "gio:%x\n", gio_dev->id.id); } static DEVICE_ATTR_RO(modalias); @@ -177,7 +176,7 @@ static ssize_t name_show(struct device *dev, struct gio_device *giodev; giodev = to_gio_device(dev); - return sprintf(buf, "%s", giodev->name); + return sysfs_emit(buf, "%s\n", giodev->name); } static DEVICE_ATTR_RO(name); @@ -187,7 +186,7 @@ static ssize_t id_show(struct device *dev, struct gio_device *giodev; giodev = to_gio_device(dev); - return sprintf(buf, "%x", giodev->id.id); + return sysfs_emit(buf, "%x\n", giodev->id.id); } static DEVICE_ATTR_RO(id);
snprintf() has the documented, but still rather strange trait of returning the length of the data that *would have been* written to the array if space were available, rather than the arguably more useful length of data *actually* written, it is usually considered wise to use scnprintf()/vscnprintf() instead. In the case of sysfs call-backs, new wrappers exist that do just that while also being PAGE_SIZE aware This patch updates the sysfs .show() callbacks to use the sysfs_emit() helper instead of snprintf() and sprintf(). Link: https://lwn.net/Articles/69419/ Link: https://github.com/KSPP/linux/issues/105 Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com> --- arch/mips/sgi-ip22/ip22-gio.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)