* [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for CONSIDER_KSTRTO
@ 2020-12-07 12:43 Aditya Srivastava
2020-12-07 12:54 ` Lukas Bulwahn
0 siblings, 1 reply; 4+ messages in thread
From: Aditya Srivastava @ 2020-12-07 12:43 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: linux-kernel-mentees, yashsri421
Checkpatch recommends the usage of kstrto* over simple_strto* and
strict_strto*, as these are obsolete.
E.g., running checkpatch on commit f79b3f338564 ("ftrace: Allow
enabling of filters via index of available_filter_functions") reports
this warning:
WARNING: simple_strtoul is obsolete, use kstrtoul instead
+ long index = simple_strtoul(func_g->search, NULL, 0);
Provide a fix by replacing simple_strto* and strict_strto* with kstrto*
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
scripts/checkpatch.pl | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7b086d1cd6c2..ca5bb5a3f8f6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6980,8 +6980,12 @@ sub process {
# recommend kstrto* over simple_strto* and strict_strto*
if ($line =~ /\b((simple|strict)_(strto(l|ll|ul|ull)))\s*\(/) {
- WARN("CONSIDER_KSTRTO",
- "$1 is obsolete, use k$3 instead\n" . $herecurr);
+ if (WARN("CONSIDER_KSTRTO",
+ "$1 is obsolete, use k$3 instead\n" . $herecurr) &&
+ $fix) {
+ my $kstrto = "k$3";
+ $fixed[$fixlinenr] =~ s/$1/$kstrto/;
+ }
}
# check for __initcall(), use device_initcall() explicitly or more appropriate function please
--
2.17.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for CONSIDER_KSTRTO
2020-12-07 12:43 [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for CONSIDER_KSTRTO Aditya Srivastava
@ 2020-12-07 12:54 ` Lukas Bulwahn
2020-12-07 14:58 ` Aditya
0 siblings, 1 reply; 4+ messages in thread
From: Lukas Bulwahn @ 2020-12-07 12:54 UTC (permalink / raw)
To: Aditya Srivastava; +Cc: linux-kernel-mentees
On Mon, Dec 7, 2020 at 1:43 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Checkpatch recommends the usage of kstrto* over simple_strto* and
> strict_strto*, as these are obsolete.
>
> E.g., running checkpatch on commit f79b3f338564 ("ftrace: Allow
> enabling of filters via index of available_filter_functions") reports
> this warning:
>
> WARNING: simple_strtoul is obsolete, use kstrtoul instead
> + long index = simple_strtoul(func_g->search, NULL, 0);
>
> Provide a fix by replacing simple_strto* and strict_strto* with kstrto*
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> scripts/checkpatch.pl | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7b086d1cd6c2..ca5bb5a3f8f6 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -6980,8 +6980,12 @@ sub process {
>
> # recommend kstrto* over simple_strto* and strict_strto*
> if ($line =~ /\b((simple|strict)_(strto(l|ll|ul|ull)))\s*\(/) {
> - WARN("CONSIDER_KSTRTO",
> - "$1 is obsolete, use k$3 instead\n" . $herecurr);
> + if (WARN("CONSIDER_KSTRTO",
> + "$1 is obsolete, use k$3 instead\n" . $herecurr) &&
> + $fix) {
> + my $kstrto = "k$3";
> + $fixed[$fixlinenr] =~ s/$1/$kstrto/;
> + }
> }
>
Does this only warn on added lines?
The fix looks good; I just do not know if the rule is applicable in
full generality.
You might want to check a few instances if that rule actually makes
sense and is accepted.
Lukas
> # check for __initcall(), use device_initcall() explicitly or more appropriate function please
> --
> 2.17.1
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for CONSIDER_KSTRTO
2020-12-07 12:54 ` Lukas Bulwahn
@ 2020-12-07 14:58 ` Aditya
2020-12-12 11:26 ` Aditya
0 siblings, 1 reply; 4+ messages in thread
From: Aditya @ 2020-12-07 14:58 UTC (permalink / raw)
To: Lukas Bulwahn; +Cc: linux-kernel-mentees
On 7/12/20 6:24 pm, Lukas Bulwahn wrote:
> On Mon, Dec 7, 2020 at 1:43 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>>
>> Checkpatch recommends the usage of kstrto* over simple_strto* and
>> strict_strto*, as these are obsolete.
>>
>> E.g., running checkpatch on commit f79b3f338564 ("ftrace: Allow
>> enabling of filters via index of available_filter_functions") reports
>> this warning:
>>
>> WARNING: simple_strtoul is obsolete, use kstrtoul instead
>> + long index = simple_strtoul(func_g->search, NULL, 0);
>>
>> Provide a fix by replacing simple_strto* and strict_strto* with kstrto*
>>
>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
>> ---
>> scripts/checkpatch.pl | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 7b086d1cd6c2..ca5bb5a3f8f6 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -6980,8 +6980,12 @@ sub process {
>>
>> # recommend kstrto* over simple_strto* and strict_strto*
>> if ($line =~ /\b((simple|strict)_(strto(l|ll|ul|ull)))\s*\(/) {
>> - WARN("CONSIDER_KSTRTO",
>> - "$1 is obsolete, use k$3 instead\n" . $herecurr);
>> + if (WARN("CONSIDER_KSTRTO",
>> + "$1 is obsolete, use k$3 instead\n" . $herecurr) &&
>> + $fix) {
>> + my $kstrto = "k$3";
>> + $fixed[$fixlinenr] =~ s/$1/$kstrto/;
>> + }
>> }
>>
>
> Does this only warn on added lines?
> No, currently it does not seem to check for added lines. However, I
don't think we need to check for added lines as we are just
substituting in the line (ie no addition or removal)
> The fix looks good; I just do not know if the rule is applicable in
> full generality.
>
> You might want to check a few instances if that rule actually makes
> sense and is accepted.
>
I checked for the use of all three formats of function (ie
simple_strto*, strict_strto* and kstrto*) for all the files.
The total count of usage of kstrto* is 2854, whereas
for simple_strto* = 867
strict_strto* = 7
This is the complete list of files with their count of occurrences:
For kstrto* =
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/consider_kstrto/kstrt.txt
For simple_strto* =
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/consider_kstrto/simple_strt.txt
For strict_strto* =
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/consider_kstrto/strict_strt.txt
What do you think?
Thanks
Aditya
> Lukas
>
>> # check for __initcall(), use device_initcall() explicitly or more appropriate function please
>> --
>> 2.17.1
>>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for CONSIDER_KSTRTO
2020-12-07 14:58 ` Aditya
@ 2020-12-12 11:26 ` Aditya
0 siblings, 0 replies; 4+ messages in thread
From: Aditya @ 2020-12-12 11:26 UTC (permalink / raw)
To: Lukas Bulwahn; +Cc: linux-kernel-mentees
On 7/12/20 8:28 pm, Aditya wrote:
> On 7/12/20 6:24 pm, Lukas Bulwahn wrote:
>> On Mon, Dec 7, 2020 at 1:43 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>>>
>>> Checkpatch recommends the usage of kstrto* over simple_strto* and
>>> strict_strto*, as these are obsolete.
>>>
>>> E.g., running checkpatch on commit f79b3f338564 ("ftrace: Allow
>>> enabling of filters via index of available_filter_functions") reports
>>> this warning:
>>>
>>> WARNING: simple_strtoul is obsolete, use kstrtoul instead
>>> + long index = simple_strtoul(func_g->search, NULL, 0);
>>>
>>> Provide a fix by replacing simple_strto* and strict_strto* with kstrto*
>>>
>>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
>>> ---
>>> scripts/checkpatch.pl | 8 ++++++--
>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index 7b086d1cd6c2..ca5bb5a3f8f6 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -6980,8 +6980,12 @@ sub process {
>>>
>>> # recommend kstrto* over simple_strto* and strict_strto*
>>> if ($line =~ /\b((simple|strict)_(strto(l|ll|ul|ull)))\s*\(/) {
>>> - WARN("CONSIDER_KSTRTO",
>>> - "$1 is obsolete, use k$3 instead\n" . $herecurr);
>>> + if (WARN("CONSIDER_KSTRTO",
>>> + "$1 is obsolete, use k$3 instead\n" . $herecurr) &&
>>> + $fix) {
>>> + my $kstrto = "k$3";
>>> + $fixed[$fixlinenr] =~ s/$1/$kstrto/;
>>> + }
>>> }
>>>
>>
>> Does this only warn on added lines?
>> No, currently it does not seem to check for added lines. However, I
> don't think we need to check for added lines as we are just
> substituting in the line (ie no addition or removal)
>
>> The fix looks good; I just do not know if the rule is applicable in
>> full generality.
>>
>> You might want to check a few instances if that rule actually makes
>> sense and is accepted.
>>
> I checked for the use of all three formats of function (ie
> simple_strto*, strict_strto* and kstrto*) for all the files.
>
> The total count of usage of kstrto* is 2854, whereas
> for simple_strto* = 867
> strict_strto* = 7
>
> This is the complete list of files with their count of occurrences:
>
> For kstrto* =
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/consider_kstrto/kstrt.txt
>
> For simple_strto* =
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/consider_kstrto/simple_strt.txt
>
> For strict_strto* =
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/consider_kstrto/strict_strt.txt
>
> What do you think?
>
Hi Lukas
You probably missed this mail.
Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-12 11:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-07 12:43 [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for CONSIDER_KSTRTO Aditya Srivastava
2020-12-07 12:54 ` Lukas Bulwahn
2020-12-07 14:58 ` Aditya
2020-12-12 11:26 ` Aditya
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.