drivers/cpuidle/sysfs.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-)
From: Vivek Yadav <vivekyadav1207731111@gmail.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@perches.com>
Signed-off-by: Vivek Yadav <vivekyadav1207731111@gmail.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;
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);
}
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;
--
2.25.1On Mon, Aug 25, 2025 at 8:58 PM <vivekyadav1207731111@gmail.com> wrote:
>
> From: Vivek Yadav <vivekyadav1207731111@gmail.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@perches.com>
> Signed-off-by: Vivek Yadav <vivekyadav1207731111@gmail.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;
> --
On Mon, Aug 25, 2025 at 8:58 PM <vivekyadav1207731111@gmail.com> wrote:
>>
>> From: Vivek Yadav <vivekyadav1207731111@gmail.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@perches.com>
>> Signed-off-by: Vivek Yadav <vivekyadav1207731111@gmail.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 ->
There is no harm if we leave the variable name as 'i' but it would be better
if we give it a suitable name like 'offset'. It will definitely improve
readability.
>
>> 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?
I think 'len' is also not a good name. Now I have two options.
Either I can remove this part from the patch or I can make new patch
where I can change the variable name 'i' into a more meaningful name
like 'offset'.
Let me know in which direction I should move.
~Vivek
On Fri, Sep 5, 2025 at 1:05 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Mon, Aug 25, 2025 at 8:58 PM <vivekyadav1207731111@gmail.com> wrote:
> >
> > From: Vivek Yadav <vivekyadav1207731111@gmail.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@perches.com>
> > Signed-off-by: Vivek Yadav <vivekyadav1207731111@gmail.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;
> > --
On Fri, 2025-09-05 at 22:57 +0530, vivek yadav wrote:
> On Mon, Aug 25, 2025 at 8:58 PM <vivekyadav1207731111@gmail.com> wrote:
> > >
> > > From: Vivek Yadav <vivekyadav1207731111@gmail.com>
> > >
> > > The ->show() callbacks in sysfs should use sysfs_emit() or
> > > sysfs_emit_at()
[]
> > > diff --git 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 ->
>
> There is no harm if we leave the variable name as 'i' but it would be better
> if we give it a suitable name like 'offset'. It will definitely improve
> readability.
size and len are most commonly used.
I prefer len.
$ git grep -P -oh '\w+\s+\+=\s*sysfs_emit_at' | \
sort | uniq -c | sort -rn
361 size += sysfs_emit_at
187 len += sysfs_emit_at
144 n += sysfs_emit_at
69 count += sysfs_emit_at
60 rc += sysfs_emit_at
51 offset += sysfs_emit_at
43 ret += sysfs_emit_at
27 rb += sysfs_emit_at
16 res += sysfs_emit_at
7 pos += sysfs_emit_at
7 i += sysfs_emit_at
5 length += sysfs_emit_at
4 used += sysfs_emit_at
4 nchars += sysfs_emit_at
4 at += sysfs_emit_at
3 sz += sysfs_emit_at
3 l += sysfs_emit_at
3 buf_len += sysfs_emit_at
2 status += sysfs_emit_at
2 idx += sysfs_emit_at
1 n_written += sysfs_emit_at
1 cnt += sysfs_emit_at
On Fri, Sep 5, 2025 at 8:22 PM Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2025-09-05 at 22:57 +0530, vivek yadav wrote:
> > On Mon, Aug 25, 2025 at 8:58 PM <vivekyadav1207731111@gmail.com> wrote:
> > > >
> > > > From: Vivek Yadav <vivekyadav1207731111@gmail.com>
> > > >
> > > > The ->show() callbacks in sysfs should use sysfs_emit() or
> > > > sysfs_emit_at()
> []
> > > > diff --git 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 ->
> >
> > There is no harm if we leave the variable name as 'i' but it would be better
> > if we give it a suitable name like 'offset'. It will definitely improve
> > readability.
>
> size and len are most commonly used.
> I prefer len.
Fine, in new code, use whatever you like, but what really is the
reason for doing a rename in code that has been almost unchanged since
2.6.22?
On Fri, 2025-09-05 at 20:34 +0200, Rafael J. Wysocki wrote:
> On Fri, Sep 5, 2025 at 8:22 PM Joe Perches <joe@perches.com> wrote:
> >
> > On Fri, 2025-09-05 at 22:57 +0530, vivek yadav wrote:
> > > On Mon, Aug 25, 2025 at 8:58 PM <vivekyadav1207731111@gmail.com> wrote:
> > > > >
> > > > > From: Vivek Yadav <vivekyadav1207731111@gmail.com>
> > > > >
> > > > > The ->show() callbacks in sysfs should use sysfs_emit() or
> > > > > sysfs_emit_at()
> > []
> > > > > diff --git 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 ->
> > >
> > > There is no harm if we leave the variable name as 'i' but it would be better
> > > if we give it a suitable name like 'offset'. It will definitely improve
> > > readability.
> >
> > size and len are most commonly used.
> > I prefer len.
>
> Fine, in new code, use whatever you like, but what really is the
> reason for doing a rename in code that has been almost unchanged since
> 2.6.22?
If a sprintf -> sysfs_emit conversion is done, it's IMO better
style to be consistent with the typical sysfs_emit uses.
>On Fri, Sep 5, 2025 at 8:22 PM Joe Perches <joe@perches.com> wrote:
>>
>> On Fri, 2025-09-05 at 22:57 +0530, vivek yadav wrote:
>> > On Mon, Aug 25, 2025 at 8:58 PM <vivekyadav1207731111@gmail.com> wrote:
>> > > >
>> > > > From: Vivek Yadav <vivekyadav1207731111@gmail.com>
>> > > >
>> > > > The ->show() callbacks in sysfs should use sysfs_emit() or
>> > > > sysfs_emit_at()
>> []
>> > > > diff --git 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 ->
>> >
>> > There is no harm if we leave the variable name as 'i' but it would be better
>> > if we give it a suitable name like 'offset'. It will definitely improve
>> > readability.
>>
>> size and len are most commonly used.
>> I prefer len.
>
>Fine, in new code, use whatever you like, but what really is the
>reason for doing a rename in code that has been almost unchanged since
>2.6.22?
Hi Rafael,
You are correct that the variable name ‘i’ has remained unchanged
since v2.6.22. Its long-standing presence is understandable, but it
doesn’t necessarily mean it can’t be updated in the future if needed.
As @Joe pointed out, statistics show that most developers prefer using
more descriptive names such as ‘len’ or ‘size’ when possible.
With this in mind, I believe it’s time to bring this discussion to a
conclusion. Shall we move forward with this change in variable naming
— YES or NO?
Looking forward to your input.
Best regards,
Vivek
On Sat, Sep 6, 2025 at 12:43 PM Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2025-09-05 at 20:34 +0200, Rafael J. Wysocki wrote:
> > On Fri, Sep 5, 2025 at 8:22 PM Joe Perches <joe@perches.com> wrote:
> > >
> > > On Fri, 2025-09-05 at 22:57 +0530, vivek yadav wrote:
> > > > On Mon, Aug 25, 2025 at 8:58 PM <vivekyadav1207731111@gmail.com> wrote:
> > > > > >
> > > > > > From: Vivek Yadav <vivekyadav1207731111@gmail.com>
> > > > > >
> > > > > > The ->show() callbacks in sysfs should use sysfs_emit() or
> > > > > > sysfs_emit_at()
> > > []
> > > > > > diff --git 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 ->
> > > >
> > > > There is no harm if we leave the variable name as 'i' but it would be better
> > > > if we give it a suitable name like 'offset'. It will definitely improve
> > > > readability.
> > >
> > > size and len are most commonly used.
> > > I prefer len.
> >
> > Fine, in new code, use whatever you like, but what really is the
> > reason for doing a rename in code that has been almost unchanged since
> > 2.6.22?
>
> If a sprintf -> sysfs_emit conversion is done, it's IMO better
> style to be consistent with the typical sysfs_emit uses.
Hi @Rafael J. Wysocki ,
Is there any update for me on this patch ?
~~Vivek
On Sat, Sep 6, 2025 at 10:52 PM vivek yadav
<vivekyadav1207731111@gmail.com> wrote:
>
> >On Fri, Sep 5, 2025 at 8:22 PM Joe Perches <joe@perches.com> wrote:
> >>
> >> On Fri, 2025-09-05 at 22:57 +0530, vivek yadav wrote:
> >> > On Mon, Aug 25, 2025 at 8:58 PM <vivekyadav1207731111@gmail.com> wrote:
> >> > > >
> >> > > > From: Vivek Yadav <vivekyadav1207731111@gmail.com>
> >> > > >
> >> > > > The ->show() callbacks in sysfs should use sysfs_emit() or
> >> > > > sysfs_emit_at()
> >> []
> >> > > > diff --git 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 ->
> >> >
> >> > There is no harm if we leave the variable name as 'i' but it would be better
> >> > if we give it a suitable name like 'offset'. It will definitely improve
> >> > readability.
> >>
> >> size and len are most commonly used.
> >> I prefer len.
> >
> >Fine, in new code, use whatever you like, but what really is the
> >reason for doing a rename in code that has been almost unchanged since
> >2.6.22?
>
> Hi Rafael,
>
> You are correct that the variable name ‘i’ has remained unchanged
> since v2.6.22. Its long-standing presence is understandable, but it
> doesn’t necessarily mean it can’t be updated in the future if needed.
>
> As @Joe pointed out, statistics show that most developers prefer using
> more descriptive names such as ‘len’ or ‘size’ when possible.
>
> With this in mind, I believe it’s time to bring this discussion to a
> conclusion. Shall we move forward with this change in variable naming
> — YES or NO?
>
> Looking forward to your input.
>
> Best regards,
> Vivek
>
> On Sat, Sep 6, 2025 at 12:43 PM Joe Perches <joe@perches.com> wrote:
> >
> > On Fri, 2025-09-05 at 20:34 +0200, Rafael J. Wysocki wrote:
> > > On Fri, Sep 5, 2025 at 8:22 PM Joe Perches <joe@perches.com> wrote:
> > > >
> > > > On Fri, 2025-09-05 at 22:57 +0530, vivek yadav wrote:
> > > > > On Mon, Aug 25, 2025 at 8:58 PM <vivekyadav1207731111@gmail.com> wrote:
> > > > > > >
> > > > > > > From: Vivek Yadav <vivekyadav1207731111@gmail.com>
> > > > > > >
> > > > > > > The ->show() callbacks in sysfs should use sysfs_emit() or
> > > > > > > sysfs_emit_at()
> > > > []
> > > > > > > diff --git 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 ->
> > > > >
> > > > > There is no harm if we leave the variable name as 'i' but it would be better
> > > > > if we give it a suitable name like 'offset'. It will definitely improve
> > > > > readability.
> > > >
> > > > size and len are most commonly used.
> > > > I prefer len.
> > >
> > > Fine, in new code, use whatever you like, but what really is the
> > > reason for doing a rename in code that has been almost unchanged since
> > > 2.6.22?
> >
> > If a sprintf -> sysfs_emit conversion is done, it's IMO better
> > style to be consistent with the typical sysfs_emit uses.
On Sun, Sep 14, 2025 at 5:20 PM vivek yadav <vivekyadav1207731111@gmail.com> wrote: > > Hi @Rafael J. Wysocki , > > Is there any update for me on this patch ? As I said before, please do not rename the variable. Thanks!
From: Vivek Yadav <vivekyadav1207731111@gmail.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@perches.com>
Signed-off-by: Vivek Yadav <vivekyadav1207731111@gmail.com>
---
drivers/cpuidle/sysfs.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index d6f5da61cb7d..61de64817604 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -27,14 +27,14 @@ static ssize_t show_available_governors(struct device *dev,
mutex_lock(&cpuidle_lock);
list_for_each_entry(tmp, &cpuidle_governors, governor_list) {
- if (i >= (ssize_t) (PAGE_SIZE - (CPUIDLE_NAME_LEN + 2)))
+ if (i >= (ssize_t)(PAGE_SIZE - (CPUIDLE_NAME_LEN + 2)))
goto out;
- i += scnprintf(&buf[i], CPUIDLE_NAME_LEN + 1, "%s ", tmp->name);
+ i += sysfs_emit_at(buf, i, "%.*s ", CPUIDLE_NAME_LEN, tmp->name);
}
out:
- i+= sprintf(&buf[i], "\n");
+ i += sysfs_emit_at(buf, i, "\n");
mutex_unlock(&cpuidle_lock);
return i;
}
@@ -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;
--
2.43.0© 2016 - 2025 Red Hat, Inc.