On Sun, 2020-11-01 at 20:48 +0000, Matthew Wilcox wrote:
> On Sun, Nov 01, 2020 at 12:12:51PM -0800, Joe Perches wrote:
> > @@ -4024,7 +4024,7 @@ int __init shmem_init(void)
> >
> >
> > #if defined(CONFIG_TRANSPARENT_HUGEPAGE) && defined(CONFIG_SYSFS)
> > static ssize_t shmem_enabled_show(struct kobject *kobj,
> > - struct kobj_attribute *attr, char *buf)
> > + struct kobj_attribute *attr, char *buf)
> > {
> > static const int values[] = {
> > SHMEM_HUGE_ALWAYS,
>
> Why?
why what?
> > @@ -4034,16 +4034,19 @@ static ssize_t shmem_enabled_show(struct kobject *kobj,
> > SHMEM_HUGE_DENY,
> > SHMEM_HUGE_FORCE,
> > };
> > - int i, count;
> > -
> > - for (i = 0, count = 0; i < ARRAY_SIZE(values); i++) {
> > - const char *fmt = shmem_huge == values[i] ? "[%s] " : "%s ";
> > + int len = 0;
> > + int i;
>
> Better:
> int i, len = 0;
I generally disagree as I think it better to have each declaration on an
individual line.
> > - count += sprintf(buf + count, fmt,
> > - shmem_format_huge(values[i]));
> > + for (i = 0; i < ARRAY_SIZE(values); i++) {
> > + len += sysfs_emit_at(buf, len,
> > + shmem_huge == values[i] ? "%s[%s]" : "%s%s",
> > + i ? " " : "",
> > + shmem_format_huge(values[i]));
>
> This is ... complicated. I thought the point of doing all the sysfs_emit
> stuff was to simplify things.
The removal of fmt allows the format and argument to be __printf verified.
Indirected formats do not generally allow that.
And using sysfs_emit is not really intended to simplify output code, it's
used to make sure there isn't a overflow of the PAGE_SIZE output buf when
using sprintf/snprintf.
