2024-01-16 04:12:41

by Li Zhijian

[permalink] [raw]
Subject: [PATCH 01/42] coccinelle: device_attr_show.cocci: update description and warning message

Update them according to latest Documentation/filesystems/sysfs.rst.

CC: Julia Lawall <[email protected]>
CC: Nicolas Palix <[email protected]>
CC: [email protected]
Signed-off-by: Li Zhijian <[email protected]>
---
scripts/coccinelle/api/device_attr_show.cocci | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/scripts/coccinelle/api/device_attr_show.cocci b/scripts/coccinelle/api/device_attr_show.cocci
index a28dc061653a..a621e9610479 100644
--- a/scripts/coccinelle/api/device_attr_show.cocci
+++ b/scripts/coccinelle/api/device_attr_show.cocci
@@ -1,10 +1,8 @@
// SPDX-License-Identifier: GPL-2.0-only
///
/// From Documentation/filesystems/sysfs.rst:
-/// show() must not use snprintf() when formatting the value to be
-/// returned to user space. If you can guarantee that an overflow
-/// will never happen you can use sprintf() otherwise you must use
-/// scnprintf().
+/// show() should only use sysfs_emit() or sysfs_emit_at() when formatting
+/// the value to be returned to user space.
///
// Confidence: High
// Copyright: (C) 2020 Denis Efremov ISPRAS
@@ -46,10 +44,10 @@ ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
p << r.p;
@@

-coccilib.report.print_report(p[0], "WARNING: use scnprintf or sprintf")
+coccilib.report.print_report(p[0], "WARNING: please use sysfs_emit")

@script: python depends on org@
p << r.p;
@@

-coccilib.org.print_todo(p[0], "WARNING: use scnprintf or sprintf")
+coccilib.org.print_todo(p[0], "WARNING: please use sysfs_emit")
--
2.29.2



2024-01-18 20:27:12

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH 01/42] coccinelle: device_attr_show.cocci: update description and warning message



On Tue, 16 Jan 2024, Li Zhijian wrote:

> Update them according to latest Documentation/filesystems/sysfs.rst.
>
> CC: Julia Lawall <[email protected]>
> CC: Nicolas Palix <[email protected]>
> CC: [email protected]
> Signed-off-by: Li Zhijian <[email protected]>
> ---
> scripts/coccinelle/api/device_attr_show.cocci | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/coccinelle/api/device_attr_show.cocci b/scripts/coccinelle/api/device_attr_show.cocci
> index a28dc061653a..a621e9610479 100644
> --- a/scripts/coccinelle/api/device_attr_show.cocci
> +++ b/scripts/coccinelle/api/device_attr_show.cocci
> @@ -1,10 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0-only
> ///
> /// From Documentation/filesystems/sysfs.rst:
> -/// show() must not use snprintf() when formatting the value to be
> -/// returned to user space. If you can guarantee that an overflow
> -/// will never happen you can use sprintf() otherwise you must use
> -/// scnprintf().
> +/// show() should only use sysfs_emit() or sysfs_emit_at() when formatting
> +/// the value to be returned to user space.
> ///
> // Confidence: High
> // Copyright: (C) 2020 Denis Efremov ISPRAS
> @@ -46,10 +44,10 @@ ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
> p << r.p;
> @@
>
> -coccilib.report.print_report(p[0], "WARNING: use scnprintf or sprintf")
> +coccilib.report.print_report(p[0], "WARNING: please use sysfs_emit")
>
> @script: python depends on org@
> p << r.p;
> @@
>
> -coccilib.org.print_todo(p[0], "WARNING: use scnprintf or sprintf")
> +coccilib.org.print_todo(p[0], "WARNING: please use sysfs_emit")

Thanks for the suggestion, but it's not really consistent, because the
patch rule still generates a call to scnprintf. Would it be possible to
fix that up? Or should it be removed?

thanks,
julia

2024-01-19 02:54:07

by Li Zhijian

[permalink] [raw]
Subject: Re: [PATCH 01/42] coccinelle: device_attr_show.cocci: update description and warning message



On 19/01/2024 04:26, Julia Lawall wrote:
>
>
> On Tue, 16 Jan 2024, Li Zhijian wrote:
>
>> Update them according to latest Documentation/filesystems/sysfs.rst.
>>
>> CC: Julia Lawall <[email protected]>
>> CC: Nicolas Palix <[email protected]>
>> CC: [email protected]
>> Signed-off-by: Li Zhijian <[email protected]>
>> ---
>> scripts/coccinelle/api/device_attr_show.cocci | 10 ++++------
>> 1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/scripts/coccinelle/api/device_attr_show.cocci b/scripts/coccinelle/api/device_attr_show.cocci
>> index a28dc061653a..a621e9610479 100644
>> --- a/scripts/coccinelle/api/device_attr_show.cocci
>> +++ b/scripts/coccinelle/api/device_attr_show.cocci
>> @@ -1,10 +1,8 @@
>> // SPDX-License-Identifier: GPL-2.0-only
>> ///
>> /// From Documentation/filesystems/sysfs.rst:
>> -/// show() must not use snprintf() when formatting the value to be
>> -/// returned to user space. If you can guarantee that an overflow
>> -/// will never happen you can use sprintf() otherwise you must use
>> -/// scnprintf().
>> +/// show() should only use sysfs_emit() or sysfs_emit_at() when formatting
>> +/// the value to be returned to user space.
>> ///
>> // Confidence: High
>> // Copyright: (C) 2020 Denis Efremov ISPRAS
>> @@ -46,10 +44,10 @@ ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
>> p << r.p;
>> @@
>>
>> -coccilib.report.print_report(p[0], "WARNING: use scnprintf or sprintf")
>> +coccilib.report.print_report(p[0], "WARNING: please use sysfs_emit")
>>
>> @script: python depends on org@
>> p << r.p;
>> @@
>>
>> -coccilib.org.print_todo(p[0], "WARNING: use scnprintf or sprintf")
>> +coccilib.org.print_todo(p[0], "WARNING: please use sysfs_emit")
>
> Thanks for the suggestion, but it's not really consistent, because the
> patch rule still generates a call to scnprintf. Would it be possible to
> fix that up? Or should it be removed?

Good catch, i missed it before.

Let's remove it? Just simply replacing scnprintf to sysfs_emit is
not enough for the patch method. Because snprintf() vs sysfs_emit()
take different arguments.

I'm not familiar with .cocci, if you know how to write the patch method,
please let me know.


Thanks
Zhijian



>
> thanks,
> julia

2024-01-19 06:27:31

by Li Zhijian

[permalink] [raw]
Subject: Re: [PATCH 01/42] coccinelle: device_attr_show.cocci: update description and warning message

Julia,


Just Learned coccinelle from this video https://www.youtube.com/watch?v=16wUxqDf1GA
and post V2[1] to fix MODE=patch, please take another look.

[1] https://lore.kernel.org/lkml/[email protected]/T/#u

Thanks
Zhijian



On 19/01/2024 10:53, Li Zhijian wrote:
>
>
> On 19/01/2024 04:26, Julia Lawall wrote:
>>
>>
>> On Tue, 16 Jan 2024, Li Zhijian wrote:
>>
>>> Update them according to latest Documentation/filesystems/sysfs.rst.
>>>
>>> CC: Julia Lawall <[email protected]>
>>> CC: Nicolas Palix <[email protected]>
>>> CC: [email protected]
>>> Signed-off-by: Li Zhijian <[email protected]>
>>> ---
>>>   scripts/coccinelle/api/device_attr_show.cocci | 10 ++++------
>>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/scripts/coccinelle/api/device_attr_show.cocci b/scripts/coccinelle/api/device_attr_show.cocci
>>> index a28dc061653a..a621e9610479 100644
>>> --- a/scripts/coccinelle/api/device_attr_show.cocci
>>> +++ b/scripts/coccinelle/api/device_attr_show.cocci
>>> @@ -1,10 +1,8 @@
>>>   // SPDX-License-Identifier: GPL-2.0-only
>>>   ///
>>>   /// From Documentation/filesystems/sysfs.rst:
>>> -///  show() must not use snprintf() when formatting the value to be
>>> -///  returned to user space. If you can guarantee that an overflow
>>> -///  will never happen you can use sprintf() otherwise you must use
>>> -///  scnprintf().
>>> +/// show() should only use sysfs_emit() or sysfs_emit_at() when formatting
>>> +/// the value to be returned to user space.
>>>   ///
>>>   // Confidence: High
>>>   // Copyright: (C) 2020 Denis Efremov ISPRAS
>>> @@ -46,10 +44,10 @@ ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
>>>   p << r.p;
>>>   @@
>>>
>>> -coccilib.report.print_report(p[0], "WARNING: use scnprintf or sprintf")
>>> +coccilib.report.print_report(p[0], "WARNING: please use sysfs_emit")
>>>
>>>   @script: python depends on org@
>>>   p << r.p;
>>>   @@
>>>
>>> -coccilib.org.print_todo(p[0], "WARNING: use scnprintf or sprintf")
>>> +coccilib.org.print_todo(p[0], "WARNING: please use sysfs_emit")
>>
>> Thanks for the suggestion, but it's not really consistent, because the
>> patch rule still generates a call to scnprintf.  Would it be possible to
>> fix that up?  Or should it be removed?
>
> Good catch, i missed it before.
>
> Let's remove it?  Just simply replacing scnprintf to sysfs_emit is
> not enough for the patch method. Because snprintf() vs sysfs_emit()
> take different arguments.
>
> I'm not familiar with .cocci, if you know how to write the patch method,
> please let me know.
>
>
> Thanks
> Zhijian
>
>
>
>>
>> thanks,
>> julia