[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <7ed56f10-99f9-41b9-9d82-45797914f3d1@wanadoo.fr>
Date: Tue, 30 Jul 2024 18:24:44 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Xueqin Luo <luoxueqin@...inos.cn>, rafael@...nel.org, pavel@....cz,
len.brown@...el.com
Cc: linux-pm@...r.kernel.org, xiongxin@...inos.cn,
linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH 2/2] PM: Use sysfs_emit() and sysfs_emit_at() in
"show" functions
Le 30/07/2024 à 08:54, Xueqin Luo a écrit :
> As Documentation/filesystems/sysfs.rst suggested,
> show() should only use sysfs_emit() or sysfs_emit_at() when formatting
> the value to be returned to user space.
>
> No functional change intended.
>
> Signed-off-by: Xueqin Luo <luoxueqin@...inos.cn>
> ---
Hi,
> @@ -149,17 +149,17 @@ static ssize_t mem_sleep_show(struct kobject *kobj, struct kobj_attribute *attr,
> const char *label = mem_sleep_states[i];
>
> if (mem_sleep_current == i)
> - s += sprintf(s, "[%s] ", label);
> + sz += sysfs_emit_at(buf, sz, "[%s] ", label);
> else
> - s += sprintf(s, "%s ", label);
> + sz += sysfs_emit_at(buf, sz, "%s ", label);
> }
> }
>
> /* Convert the last space to a newline if needed. */
> - if (s != buf)
> - *(s-1) = '\n';
The goal here is to remove the trailing space. So you slightly change
the behavior.
Not sure if it matters or not (this was not done in disk_show() in patch
1/2).
Maybe an (ugly)
if (sz)
sz += sysfs_emit_at(buf, sz - 1, "\n");
or something like:
if (sz) {
sz--;
sz += sysfs_emit_at(buf, sz, "\n");
}
> + if (sz)
> + sz += sysfs_emit_at(buf, sz, "\n");
>
> - return (s - buf);
> + return sz;
> }
>
> static suspend_state_t decode_suspend_state(const char *buf, size_t n)
...
> @@ -257,22 +257,22 @@ static const char * const pm_tests[__TEST_AFTER_LAST] = {
> static ssize_t pm_test_show(struct kobject *kobj, struct kobj_attribute *attr,
> char *buf)
> {
> - char *s = buf;
> int level;
> + size_t sz = 0;
>
> for (level = TEST_FIRST; level <= TEST_MAX; level++)
> if (pm_tests[level]) {
> if (level == pm_test_level)
> - s += sprintf(s, "[%s] ", pm_tests[level]);
> + sz += sysfs_emit_at(buf, sz, "[%s] ", pm_tests[level]);
> else
> - s += sprintf(s, "%s ", pm_tests[level]);
> + sz += sysfs_emit_at(buf, sz, "%s ", pm_tests[level]);
> }
>
> - if (s != buf)
> + if (sz)
> /* convert the last space to a newline */
> - *(s-1) = '\n';
> + sz += sysfs_emit_at(buf, sz, "\n");
Same here.
>
> - return (s - buf);
> + return sz;
> }
>
> static ssize_t pm_test_store(struct kobject *kobj, struct kobj_attribute *attr,
> @@ -390,7 +390,7 @@ static const char * const suspend_step_names[] = {
> static ssize_t _name##_show(struct kobject *kobj, \
> struct kobj_attribute *attr, char *buf) \
> { \
> - return sprintf(buf, format_str, suspend_stats._name); \
> + return sysfs_emit(buf, format_str, suspend_stats._name); \
Maybe 1 tab should be removed?
> } \
> static struct kobj_attribute _name = __ATTR_RO(_name)
>
...
> @@ -668,21 +668,21 @@ struct kobject *power_kobj;
> static ssize_t state_show(struct kobject *kobj, struct kobj_attribute *attr,
> char *buf)
> {
> - char *s = buf;
> + ssize_t sz = 0;
> #ifdef CONFIG_SUSPEND
> suspend_state_t i;
>
> for (i = PM_SUSPEND_MIN; i < PM_SUSPEND_MAX; i++)
> if (pm_states[i])
> - s += sprintf(s,"%s ", pm_states[i]);
> + sz += sysfs_emit_at(buf, sz, "%s ", pm_states[i]);
>
> #endif
> if (hibernation_available())
> - s += sprintf(s, "disk ");
> - if (s != buf)
> + sz += sysfs_emit_at(buf, sz, "disk ");
> + if (sz)
> /* convert the last space to a newline */
> - *(s-1) = '\n';
> - return (s - buf);
> + sz += sysfs_emit_at(buf, sz, "\n");
Same here
> + return sz;
> }
>
> static suspend_state_t decode_state(const char *buf, size_t n)
...
CJ
Powered by blists - more mailing lists