* [PATCH linux-next] genirq: use sysfs_emit() instead of scnprintf().
@ 2025-03-15 6:17 xie.ludan
2025-03-17 6:07 ` Jiri Slaby
0 siblings, 1 reply; 6+ messages in thread
From: xie.ludan @ 2025-03-15 6:17 UTC (permalink / raw)
To: tglx; +Cc: linux-kernel
[-- Attachment #1.1.1: Type: text/plain, Size: 2042 bytes --]
From: XieLudan <xie.ludan@zte.com.cn>
Follow the advice in Documentation/filesystems/sysfs.rst:
show() should only use sysfs_emit() or sysfs_emit_at() when formatting
the value to be returned to user space.
Signed-off-by: XieLudan <xie.ludan@zte.com.cn>
---
kernel/irq/irqdesc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 287830739783..8ffe12fe5af6 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -257,11 +257,11 @@ static ssize_t per_cpu_count_show(struct kobject *kobj,
for_each_possible_cpu(cpu) {
unsigned int c = irq_desc_kstat_cpu(desc, cpu);
- ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%u", p, c);
+ ret += sysfs_emit(buf + ret, "%s%u", p, c);
p = ",";
}
- ret += scnprintf(buf + ret, PAGE_SIZE - ret, "\n");
+ ret += sysfs_emit(buf + ret, "\n");
return ret;
}
IRQ_ATTR_RO(per_cpu_count);
@@ -274,7 +274,7 @@ static ssize_t chip_name_show(struct kobject *kobj,
raw_spin_lock_irq(&desc->lock);
if (desc->irq_data.chip && desc->irq_data.chip->name) {
- ret = scnprintf(buf, PAGE_SIZE, "%s\n",
+ ret = sysfs_emit(buf, "%s\n",
desc->irq_data.chip->name);
}
raw_spin_unlock_irq(&desc->lock);
@@ -337,7 +337,7 @@ static ssize_t name_show(struct kobject *kobj,
raw_spin_lock_irq(&desc->lock);
if (desc->name)
- ret = scnprintf(buf, PAGE_SIZE, "%s\n", desc->name);
+ ret = sysfs_emit(buf, "%s\n", desc->name);
raw_spin_unlock_irq(&desc->lock);
return ret;
@@ -354,14 +354,14 @@ static ssize_t actions_show(struct kobject *kobj,
raw_spin_lock_irq(&desc->lock);
for_each_action_of_desc(desc, action) {
- ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%s",
+ ret += sysfs_emit(buf + ret, "%s%s",
p, action->name);
p = ",";
}
raw_spin_unlock_irq(&desc->lock);
if (ret)
- ret += scnprintf(buf + ret, PAGE_SIZE - ret, "\n");
+ ret += sysfs_emit(buf + ret, "\n");
return ret;
}
--
2.25.1
[-- Attachment #1.1.2: Type: text/html , Size: 4379 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH linux-next] genirq: use sysfs_emit() instead of scnprintf().
2025-03-15 6:17 [PATCH linux-next] genirq: use sysfs_emit() instead of scnprintf() xie.ludan
@ 2025-03-17 6:07 ` Jiri Slaby
2025-03-17 6:12 ` Jiri Slaby
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2025-03-17 6:07 UTC (permalink / raw)
To: xie.ludan, tglx; +Cc: linux-kernel
Hi,
your e-mail is both multipart and base64 encoded :(. Pls fix up your
setup first.
On 15. 03. 25, 7:17, xie.ludan@zte.com.cn wrote:
> From: XieLudan <xie.ludan@zte.com.cn>
>
>
> Follow the advice in Documentation/filesystems/sysfs.rst:
>
> show() should only use sysfs_emit() or sysfs_emit_at() when formatting
>
> the value to be returned to user space.
>
>
> Signed-off-by: XieLudan <xie.ludan@zte.com.cn>
>
> ---
>
> kernel/irq/irqdesc.c | 12 ++++++------
>
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>
> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
>
> index 287830739783..8ffe12fe5af6 100644
>
> --- a/kernel/irq/irqdesc.c
>
> +++ b/kernel/irq/irqdesc.c
>
> @@ -257,11 +257,11 @@ static ssize_t per_cpu_count_show(struct kobject
> *kobj,
>
> for_each_possible_cpu(cpu) {
>
> unsigned int c = irq_desc_kstat_cpu(desc, cpu);
>
> -ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%u", p, c);
>
> +ret += sysfs_emit(buf + ret, "%s%u", p, c);
Well, so the PAGE_SIZE boundary is never checked for this stacking write
now, right?
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH linux-next] genirq: use sysfs_emit() instead of scnprintf().
2025-03-17 6:07 ` Jiri Slaby
@ 2025-03-17 6:12 ` Jiri Slaby
2025-03-17 6:17 ` Jiri Slaby
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2025-03-17 6:12 UTC (permalink / raw)
To: xie.ludan, tglx; +Cc: linux-kernel
On 17. 03. 25, 7:07, Jiri Slaby wrote:
> Hi,
>
> your e-mail is both multipart and base64 encoded :(. Pls fix up your
> setup first.
>
> On 15. 03. 25, 7:17, xie.ludan@zte.com.cn wrote:
>> From: XieLudan <xie.ludan@zte.com.cn>
>>
>>
>> Follow the advice in Documentation/filesystems/sysfs.rst:
>>
>> show() should only use sysfs_emit() or sysfs_emit_at() when formatting
>>
>> the value to be returned to user space.
>>
>>
>> Signed-off-by: XieLudan <xie.ludan@zte.com.cn>
>>
>> ---
>>
>> kernel/irq/irqdesc.c | 12 ++++++------
>>
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>>
>> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
>>
>> index 287830739783..8ffe12fe5af6 100644
>>
>> --- a/kernel/irq/irqdesc.c
>>
>> +++ b/kernel/irq/irqdesc.c
>>
>> @@ -257,11 +257,11 @@ static ssize_t per_cpu_count_show(struct kobject
>> *kobj,
>>
>> for_each_possible_cpu(cpu) {
>>
>> unsigned int c = irq_desc_kstat_cpu(desc, cpu);
>>
>> -ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%u", p, c);
>>
>> +ret += sysfs_emit(buf + ret, "%s%u", p, c);
>
> Well, so the PAGE_SIZE boundary is never checked for this stacking write
> now, right?
^^^ not really clear what I mean, so:
If you advance buf by ret every time, sysfs_emit() still would only
check if buf is not written more than "PAGE_SIZE" bytes. But the
original code performs this proper "PAGE_SIZE - ret" check.
> thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH linux-next] genirq: use sysfs_emit() instead of scnprintf().
2025-03-17 6:12 ` Jiri Slaby
@ 2025-03-17 6:17 ` Jiri Slaby
0 siblings, 0 replies; 6+ messages in thread
From: Jiri Slaby @ 2025-03-17 6:17 UTC (permalink / raw)
To: xie.ludan, tglx; +Cc: linux-kernel
On 17. 03. 25, 7:12, Jiri Slaby wrote:
>>> --- a/kernel/irq/irqdesc.c
>>>
>>> +++ b/kernel/irq/irqdesc.c
>>>
>>> @@ -257,11 +257,11 @@ static ssize_t per_cpu_count_show(struct
>>> kobject *kobj,
>>>
>>> for_each_possible_cpu(cpu) {
>>>
>>> unsigned int c = irq_desc_kstat_cpu(desc, cpu);
>>>
>>> -ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%u", p, c);
>>>
>>> +ret += sysfs_emit(buf + ret, "%s%u", p, c);
>>
>> Well, so the PAGE_SIZE boundary is never checked for this stacking
>> write now, right?
>
> ^^^ not really clear what I mean, so:
>
> If you advance buf by ret every time, sysfs_emit() still would only
> check if buf is not written more than "PAGE_SIZE" bytes. But the
> original code performs this proper "PAGE_SIZE - ret" check.
Apparently, sysfs_emit_at() is your friend here.
>> thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH linux-next] genirq: use sysfs_emit() instead of scnprintf().
@ 2025-03-17 6:27 xie.ludan
2025-03-17 6:37 ` Jiri Slaby
0 siblings, 1 reply; 6+ messages in thread
From: xie.ludan @ 2025-03-17 6:27 UTC (permalink / raw)
To: tglx; +Cc: linux-kernel
Date: Fri, 14 Mar 2025 12:05:36 +0800
Follow the advice in Documentation/filesystems/sysfs.rst:
show() should only use sysfs_emit() or sysfs_emit_at() when formatting
the value to be returned to user space.
Signed-off-by: XieLudan <xie.ludan@zte.com.cn>
---
kernel/irq/irqdesc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 287830739783..8ffe12fe5af6 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -257,11 +257,11 @@ static ssize_t per_cpu_count_show(struct kobject *kobj,
for_each_possible_cpu(cpu) {
unsigned int c = irq_desc_kstat_cpu(desc, cpu);
- ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%u", p, c);
+ ret += sysfs_emit(buf + ret, "%s%u", p, c);
p = ",";
}
- ret += scnprintf(buf + ret, PAGE_SIZE - ret, "\n");
+ ret += sysfs_emit(buf + ret, "\n");
return ret;
}
IRQ_ATTR_RO(per_cpu_count);
@@ -274,7 +274,7 @@ static ssize_t chip_name_show(struct kobject *kobj,
raw_spin_lock_irq(&desc->lock);
if (desc->irq_data.chip && desc->irq_data.chip->name) {
- ret = scnprintf(buf, PAGE_SIZE, "%s\n",
+ ret = sysfs_emit(buf, "%s\n",
desc->irq_data.chip->name);
}
raw_spin_unlock_irq(&desc->lock);
@@ -337,7 +337,7 @@ static ssize_t name_show(struct kobject *kobj,
raw_spin_lock_irq(&desc->lock);
if (desc->name)
- ret = scnprintf(buf, PAGE_SIZE, "%s\n", desc->name);
+ ret = sysfs_emit(buf, "%s\n", desc->name);
raw_spin_unlock_irq(&desc->lock);
return ret;
@@ -354,14 +354,14 @@ static ssize_t actions_show(struct kobject *kobj,
raw_spin_lock_irq(&desc->lock);
for_each_action_of_desc(desc, action) {
- ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%s",
+ ret += sysfs_emit(buf + ret, "%s%s",
p, action->name);
p = ",";
}
raw_spin_unlock_irq(&desc->lock);
if (ret)
- ret += scnprintf(buf + ret, PAGE_SIZE - ret, "\n");
+ ret += sysfs_emit(buf + ret, "\n");
return ret;
}
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH linux-next] genirq: use sysfs_emit() instead of scnprintf().
2025-03-17 6:27 xie.ludan
@ 2025-03-17 6:37 ` Jiri Slaby
0 siblings, 0 replies; 6+ messages in thread
From: Jiri Slaby @ 2025-03-17 6:37 UTC (permalink / raw)
To: xie.ludan, tglx; +Cc: linux-kernel
On 17. 03. 25, 7:27, xie.ludan@zte.com.cn wrote:
> Date: Fri, 14 Mar 2025 12:05:36 +0800
>
> Follow the advice in Documentation/filesystems/sysfs.rst:
> show() should only use sysfs_emit() or sysfs_emit_at() when formatting
> the value to be returned to user space.
>
> Signed-off-by: XieLudan <xie.ludan@zte.com.cn>
I point you to study Documentation/process/submitting-patches.rst first.
In particular, I am missing versioning and versionlog.
I would also appreciate being CCed on next versions of patches I am
reviewing.
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -257,11 +257,11 @@ static ssize_t per_cpu_count_show(struct kobject *kobj,
> for_each_possible_cpu(cpu) {
> unsigned int c = irq_desc_kstat_cpu(desc, cpu);
>
> - ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%u", p, c);
> + ret += sysfs_emit(buf + ret, "%s%u", p, c);
Sadly, the more important concern I had remained.
regards,
--
js
suse labs
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-03-17 6:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-15 6:17 [PATCH linux-next] genirq: use sysfs_emit() instead of scnprintf() xie.ludan
2025-03-17 6:07 ` Jiri Slaby
2025-03-17 6:12 ` Jiri Slaby
2025-03-17 6:17 ` Jiri Slaby
2025-03-17 6:27 xie.ludan
2025-03-17 6:37 ` Jiri Slaby
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®