| Message ID | 20210421201639.1541945-1-linux@rasmusvillemoes.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Input: atkbd - simplify atkbd_show_force_release() | expand |
Hi Rasmus, On Wed, Apr 21, 2021 at 10:16:39PM +0200, Rasmus Villemoes wrote: > We can just include the newline in the format string, and scnprintf() > guarantees nul-termination. These days, sysfs_emit() is > preferred in sysfs ->show methods, so switch to that. Technically speaking the conversion is not 100% equivalent - original code ensured that there is always a newline in the output. I kind of like this, so maybe we need sysfs_emit_nl() or similar? > > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > --- > drivers/input/keyboard/atkbd.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c > index edc613efc158..b146a3ec631a 100644 > --- a/drivers/input/keyboard/atkbd.c > +++ b/drivers/input/keyboard/atkbd.c > @@ -1492,13 +1492,8 @@ static ssize_t atkbd_set_extra(struct atkbd *atkbd, const char *buf, size_t coun > > static ssize_t atkbd_show_force_release(struct atkbd *atkbd, char *buf) > { > - size_t len = scnprintf(buf, PAGE_SIZE - 1, "%*pbl", > - ATKBD_KEYMAP_SIZE, atkbd->force_release_mask); > - > - buf[len++] = '\n'; > - buf[len] = '\0'; > - > - return len; > + return sysfs_emit(buf, "%*pbl\n", > + ATKBD_KEYMAP_SIZE, atkbd->force_release_mask); > } > > static ssize_t atkbd_set_force_release(struct atkbd *atkbd, > -- > 2.29.2 > Thanks.
On 25/04/2021 04.27, Dmitry Torokhov wrote: > Hi Rasmus, > > On Wed, Apr 21, 2021 at 10:16:39PM +0200, Rasmus Villemoes wrote: >> We can just include the newline in the format string, and scnprintf() >> guarantees nul-termination. These days, sysfs_emit() is >> preferred in sysfs ->show methods, so switch to that. > > Technically speaking the conversion is not 100% equivalent - original > code ensured that there is always a newline in the output. I kind of > like this, so maybe we need sysfs_emit_nl() or similar? (1) From a UX perspective, it actually seems better to have lack of trailing newline as a (though subtle) indicator that the output was truncated (2) Given that ATKBD_KEYMAP_SIZE is 512, I don't see how that could actually ever happen. AFAICT, the worst case %pbl representation is when the bitmap consists of a series two set bits and one unset bit, yielding a pattern of "xxx-yyy," (with yyy=xxx+1) for 8 characters per 3 bits, or about 1365 characters (slight overestimate because of one- and two-digit numbers).
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c index edc613efc158..b146a3ec631a 100644 --- a/drivers/input/keyboard/atkbd.c +++ b/drivers/input/keyboard/atkbd.c @@ -1492,13 +1492,8 @@ static ssize_t atkbd_set_extra(struct atkbd *atkbd, const char *buf, size_t coun static ssize_t atkbd_show_force_release(struct atkbd *atkbd, char *buf) { - size_t len = scnprintf(buf, PAGE_SIZE - 1, "%*pbl", - ATKBD_KEYMAP_SIZE, atkbd->force_release_mask); - - buf[len++] = '\n'; - buf[len] = '\0'; - - return len; + return sysfs_emit(buf, "%*pbl\n", + ATKBD_KEYMAP_SIZE, atkbd->force_release_mask); } static ssize_t atkbd_set_force_release(struct atkbd *atkbd,
We can just include the newline in the format string, and scnprintf() guarantees nul-termination. These days, sysfs_emit() is preferred in sysfs ->show methods, so switch to that. Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- drivers/input/keyboard/atkbd.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)