| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0gfrTvLgs=PdmRbRRN05GE4Bk8Q7hJdtQfyk3VqaOz7FQ@mail.gmail.com>
Date: Thu, 4 Sep 2025 21:35:43 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: vivekyadav1207731111@...il.com
Cc: daniel.lezcano@...aro.org, rafael@...nel.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org, joe@...ches.com
Subject: Re: [PATCH] cpuidle: sysfs: Use sysfs_emit/sysfs_emit_at instead of sprintf/scnprintf
On Mon, Aug 25, 2025 at 8:58 PM <vivekyadav1207731111@...il.com> wrote:
>
> From: Vivek Yadav <vivekyadav1207731111@...il.com>
>
> The ->show() callbacks in sysfs should use sysfs_emit() or
> sysfs_emit_at() when formatting values for user space. These helpers
> are the recommended way to ensure correct buffer handling and
> consistency across the kernel.
>
> See Documentation/filesystems/sysfs.rst for details.
>
> No functional change intended.
>
> Suggested-by: Joe Perches <joe@...ches.com>
> Signed-off-by: Vivek Yadav <vivekyadav1207731111@...il.com>
> ---
> drivers/cpuidle/sysfs.c | 38 +++++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> index d6f5da61cb7d..c7af09460b74 100644
> --- a/drivers/cpuidle/sysfs.c
> +++ b/drivers/cpuidle/sysfs.c
> @@ -22,21 +22,21 @@ static ssize_t show_available_governors(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> - ssize_t i = 0;
> + ssize_t len = 0;
The variable rename is not necessary or even useful AFAICS ->
> struct cpuidle_governor *tmp;
>
> mutex_lock(&cpuidle_lock);
> list_for_each_entry(tmp, &cpuidle_governors, governor_list) {
> - if (i >= (ssize_t) (PAGE_SIZE - (CPUIDLE_NAME_LEN + 2)))
> + if (len >= (ssize_t)(PAGE_SIZE - (CPUIDLE_NAME_LEN + 2)))
> goto out;
>
> - i += scnprintf(&buf[i], CPUIDLE_NAME_LEN + 1, "%s ", tmp->name);
> + len += sysfs_emit_at(buf, len, "%.*s ", CPUIDLE_NAME_LEN, tmp->name);
-> because the second argument here is still an offset relative to
buf, isn't it?
> }
>
> out:
> - i+= sprintf(&buf[i], "\n");
> + len += sysfs_emit_at(buf, len, "\n");
> mutex_unlock(&cpuidle_lock);
> - return i;
> + return len;
> }
>
> static ssize_t show_current_driver(struct device *dev,
> @@ -49,9 +49,9 @@ static ssize_t show_current_driver(struct device *dev,
> spin_lock(&cpuidle_driver_lock);
> drv = cpuidle_get_driver();
> if (drv)
> - ret = sprintf(buf, "%s\n", drv->name);
> + ret = sysfs_emit(buf, "%s\n", drv->name);
> else
> - ret = sprintf(buf, "none\n");
> + ret = sysfs_emit(buf, "none\n");
> spin_unlock(&cpuidle_driver_lock);
>
> return ret;
> @@ -65,9 +65,9 @@ static ssize_t show_current_governor(struct device *dev,
>
> mutex_lock(&cpuidle_lock);
> if (cpuidle_curr_governor)
> - ret = sprintf(buf, "%s\n", cpuidle_curr_governor->name);
> + ret = sysfs_emit(buf, "%s\n", cpuidle_curr_governor->name);
> else
> - ret = sprintf(buf, "none\n");
> + ret = sysfs_emit(buf, "none\n");
> mutex_unlock(&cpuidle_lock);
>
> return ret;
> @@ -230,7 +230,7 @@ static struct cpuidle_state_attr attr_##_name = __ATTR(_name, 0644, show, store)
> static ssize_t show_state_##_name(struct cpuidle_state *state, \
> struct cpuidle_state_usage *state_usage, char *buf) \
> { \
> - return sprintf(buf, "%u\n", state->_name);\
> + return sysfs_emit(buf, "%u\n", state->_name);\
> }
>
> #define define_show_state_ull_function(_name) \
> @@ -238,7 +238,7 @@ static ssize_t show_state_##_name(struct cpuidle_state *state, \
> struct cpuidle_state_usage *state_usage, \
> char *buf) \
> { \
> - return sprintf(buf, "%llu\n", state_usage->_name);\
> + return sysfs_emit(buf, "%llu\n", state_usage->_name);\
> }
>
> #define define_show_state_str_function(_name) \
> @@ -247,8 +247,8 @@ static ssize_t show_state_##_name(struct cpuidle_state *state, \
> char *buf) \
> { \
> if (state->_name[0] == '\0')\
> - return sprintf(buf, "<null>\n");\
> - return sprintf(buf, "%s\n", state->_name);\
> + return sysfs_emit(buf, "<null>\n");\
> + return sysfs_emit(buf, "%s\n", state->_name);\
> }
>
> #define define_show_state_time_function(_name) \
> @@ -256,7 +256,7 @@ static ssize_t show_state_##_name(struct cpuidle_state *state, \
> struct cpuidle_state_usage *state_usage, \
> char *buf) \
> { \
> - return sprintf(buf, "%llu\n", ktime_to_us(state->_name##_ns)); \
> + return sysfs_emit(buf, "%llu\n", ktime_to_us(state->_name##_ns)); \
> }
>
> define_show_state_time_function(exit_latency)
> @@ -273,14 +273,14 @@ static ssize_t show_state_time(struct cpuidle_state *state,
> struct cpuidle_state_usage *state_usage,
> char *buf)
> {
> - return sprintf(buf, "%llu\n", ktime_to_us(state_usage->time_ns));
> + return sysfs_emit(buf, "%llu\n", ktime_to_us(state_usage->time_ns));
> }
>
> static ssize_t show_state_disable(struct cpuidle_state *state,
> struct cpuidle_state_usage *state_usage,
> char *buf)
> {
> - return sprintf(buf, "%llu\n",
> + return sysfs_emit(buf, "%llu\n",
> state_usage->disable & CPUIDLE_STATE_DISABLED_BY_USER);
> }
>
> @@ -310,7 +310,7 @@ static ssize_t show_state_default_status(struct cpuidle_state *state,
> struct cpuidle_state_usage *state_usage,
> char *buf)
> {
> - return sprintf(buf, "%s\n",
> + return sysfs_emit(buf, "%s\n",
> state->flags & CPUIDLE_FLAG_OFF ? "disabled" : "enabled");
> }
>
> @@ -358,7 +358,7 @@ static ssize_t show_state_s2idle_##_name(struct cpuidle_state *state, \
> struct cpuidle_state_usage *state_usage, \
> char *buf) \
> { \
> - return sprintf(buf, "%llu\n", state_usage->s2idle_##_name);\
> + return sysfs_emit(buf, "%llu\n", state_usage->s2idle_##_name);\
> }
>
> define_show_state_s2idle_ull_function(usage);
> @@ -550,7 +550,7 @@ static ssize_t show_driver_name(struct cpuidle_driver *drv, char *buf)
> ssize_t ret;
>
> spin_lock(&cpuidle_driver_lock);
> - ret = sprintf(buf, "%s\n", drv ? drv->name : "none");
> + ret = sysfs_emit(buf, "%s\n", drv ? drv->name : "none");
> spin_unlock(&cpuidle_driver_lock);
>
> return ret;
> --
Powered by blists - more mailing lists