[Linux-kernel-mentees] [PATCH] checkpatch: add fix option and improve msg for BOOL_COMPARISON

[Linux-kernel-mentees] [PATCH] checkpatch: add fix option and improve msg for BOOL_COMPARISON

[Aditya Srivastava]

Currently, checkpatch warns if the user compares expression with boolean.

E.g., running checkpatch on commit 3b3a1a0b7824 ("staging: rtl8723bs:
hal: Modify comparison to constant in rtl8723bs_xmit.c") reports:

CHECK: Using comparison to true is error prone
+	if (ret == true)

Provide a fix by replacing the expression with preferred expression. i.e.,

1) If the check is for a 'true' or 'not false' expression, the operand
will get replaced with empty string.

2) Else, the operand gets replaced with '!', and the arguments follow.

Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
This fix is actually inspired from the comment mentioned in the script by Joe. I have added the Suggested-by line correspondingly.

 scripts/checkpatch.pl | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8bd8e4518f7f..abd5a3d2e913 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6973,8 +6973,11 @@ sub process {
 					$op = "";
 				}
 
-				CHK("BOOL_COMPARISON",
-				    "Using comparison to $otype is error prone\n" . $herecurr);
+				if (CHK("BOOL_COMPARISON",
+					"Using comparison to '$otype' is error prone. Perhaps use '${lead}${op}${arg}${trail}'\n" . $herecurr) &&
+				    $fix) {
+					$fixed[$fixlinenr] =~ s/\b(?:true|false|$Lval)\s*(?:==|\!=)\s*(?:true|false|$Lval)\b/$op$arg/;
+				}
 
 ## maybe suggesting a correct construct would better
 ##				    "Using comparison to $otype is error prone.  Perhaps use '${lead}${op}${arg}${trail}'\n" . $herecurr);
-- 
2.17.1

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option and improve msg for BOOL_COMPARISON

[Lukas Bulwahn]

On Wed, Dec 9, 2020 at 11:58 AM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Currently, checkpatch warns if the user compares expression with boolean.
>
> E.g., running checkpatch on commit 3b3a1a0b7824 ("staging: rtl8723bs:
> hal: Modify comparison to constant in rtl8723bs_xmit.c") reports:
>
> CHECK: Using comparison to true is error prone
> +       if (ret == true)
>
> Provide a fix by replacing the expression with preferred expression. i.e.,
>
> 1) If the check is for a 'true' or 'not false' expression, the operand
> will get replaced with empty string.
>
> 2) Else, the operand gets replaced with '!', and the arguments follow.
>

Can you show the test cases you used to test this?

Can you show all findings of this rule on the current rc or linux-next?

Lukas

> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> This fix is actually inspired from the comment mentioned in the script by Joe. I have added the Suggested-by line correspondingly.
>
>  scripts/checkpatch.pl | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 8bd8e4518f7f..abd5a3d2e913 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -6973,8 +6973,11 @@ sub process {
>                                         $op = "";
>                                 }
>
> -                               CHK("BOOL_COMPARISON",
> -                                   "Using comparison to $otype is error prone\n" . $herecurr);
> +                               if (CHK("BOOL_COMPARISON",
> +                                       "Using comparison to '$otype' is error prone. Perhaps use '${lead}${op}${arg}${trail}'\n" . $herecurr) &&
> +                                   $fix) {
> +                                       $fixed[$fixlinenr] =~ s/\b(?:true|false|$Lval)\s*(?:==|\!=)\s*(?:true|false|$Lval)\b/$op$arg/;
> +                               }
>
>  ## maybe suggesting a correct construct would better
>  ##                                 "Using comparison to $otype is error prone.  Perhaps use '${lead}${op}${arg}${trail}'\n" . $herecurr);
> --
> 2.17.1
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option and improve msg for BOOL_COMPARISON

[Aditya]

On 9/12/20 5:17 pm, Lukas Bulwahn wrote:
> On Wed, Dec 9, 2020 at 11:58 AM Aditya Srivastava <yashsri421@gmail.com> wrote:
>>
>> Currently, checkpatch warns if the user compares expression with boolean.
>>
>> E.g., running checkpatch on commit 3b3a1a0b7824 ("staging: rtl8723bs:
>> hal: Modify comparison to constant in rtl8723bs_xmit.c") reports:
>>
>> CHECK: Using comparison to true is error prone
>> +       if (ret == true)
>>
>> Provide a fix by replacing the expression with preferred expression. i.e.,
>>
>> 1) If the check is for a 'true' or 'not false' expression, the operand
>> will get replaced with empty string.
>>
>> 2) Else, the operand gets replaced with '!', and the arguments follow.
>>
> 
> Can you show the test cases you used to test this?
> 
Yes, I ran it over certain commits.

Eg 1) commit 5c789e131cbb ("netfilter: nf_conncount: Add list lock and
gc worker, and RCU for init tree search"), where

1.i)
+	if (list->dead == true) {
gets changed to:
+	if (list->dead) {

1.ii)
+		if (rbconn->list.count == 0 && rbconn->list.dead == false) {
gets changed to:
+		if (rbconn->list.count == 0 && !rbconn->list.dead) {

E.g. 2) commit 83ee6ec7740b75dc0db (Staging: rtl8723bs: os_dep: Fix
if-else coding style issues) where

2.i)
+	if (padapter->bDriverStopped == true) {
gets changed to:
+	if (padapter->bDriverStopped) {

2.ii)
+	if (pwrpriv->bInSuspend == true) {
gets changed to:
+	if (pwrpriv->bInSuspend) {

2.iii)
+	if (pwrpriv->bInSuspend == false) {
gets changed to:
+	if (!pwrpriv->bInSuspend) {

> Can you show all findings of this rule on the current rc or linux-next?
> 

I have generated report on last 10k commits of latest next branch,
where the check was found to be reported 5 times.

Here are the generated msgs:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/bool_comparison/last_10k.txt

I have also generated the new CHK messages for BOOL_COMPARISON (over
v4.13..v5.8) at:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/bool_comparison/commitsv4.13..v5.8.txt

which might give us a better idea for what how the lines will be replaced.

Thanks
Aditya

> Lukas
> 
>> Suggested-by: Joe Perches <joe@perches.com>
>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
>> ---
>> This fix is actually inspired from the comment mentioned in the script by Joe. I have added the Suggested-by line correspondingly.
>>
>>  scripts/checkpatch.pl | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 8bd8e4518f7f..abd5a3d2e913 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -6973,8 +6973,11 @@ sub process {
>>                                         $op = "";
>>                                 }
>>
>> -                               CHK("BOOL_COMPARISON",
>> -                                   "Using comparison to $otype is error prone\n" . $herecurr);
>> +                               if (CHK("BOOL_COMPARISON",
>> +                                       "Using comparison to '$otype' is error prone. Perhaps use '${lead}${op}${arg}${trail}'\n" . $herecurr) &&
>> +                                   $fix) {
>> +                                       $fixed[$fixlinenr] =~ s/\b(?:true|false|$Lval)\s*(?:==|\!=)\s*(?:true|false|$Lval)\b/$op$arg/;
>> +                               }
>>
>>  ## maybe suggesting a correct construct would better
>>  ##                                 "Using comparison to $otype is error prone.  Perhaps use '${lead}${op}${arg}${trail}'\n" . $herecurr);
>> --
>> 2.17.1
>>

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option and improve msg for BOOL_COMPARISON

[Lukas Bulwahn]

On Wed, Dec 9, 2020 at 4:59 PM Aditya <yashsri421@gmail.com> wrote:
>
> On 9/12/20 5:17 pm, Lukas Bulwahn wrote:
> > On Wed, Dec 9, 2020 at 11:58 AM Aditya Srivastava <yashsri421@gmail.com> wrote:
> >>
> >> Currently, checkpatch warns if the user compares expression with boolean.
> >>
> >> E.g., running checkpatch on commit 3b3a1a0b7824 ("staging: rtl8723bs:
> >> hal: Modify comparison to constant in rtl8723bs_xmit.c") reports:
> >>
> >> CHECK: Using comparison to true is error prone
> >> +       if (ret == true)
> >>
> >> Provide a fix by replacing the expression with preferred expression. i.e.,
> >>
> >> 1) If the check is for a 'true' or 'not false' expression, the operand
> >> will get replaced with empty string.
> >>
> >> 2) Else, the operand gets replaced with '!', and the arguments follow.
> >>
> >
> > Can you show the test cases you used to test this?
> >
> Yes, I ran it over certain commits.
>
> Eg 1) commit 5c789e131cbb ("netfilter: nf_conncount: Add list lock and
> gc worker, and RCU for init tree search"), where
>
> 1.i)
> +       if (list->dead == true) {
> gets changed to:
> +       if (list->dead) {
>
> 1.ii)
> +               if (rbconn->list.count == 0 && rbconn->list.dead == false) {
> gets changed to:
> +               if (rbconn->list.count == 0 && !rbconn->list.dead) {
>
> E.g. 2) commit 83ee6ec7740b75dc0db (Staging: rtl8723bs: os_dep: Fix
> if-else coding style issues) where
>
> 2.i)
> +       if (padapter->bDriverStopped == true) {
> gets changed to:
> +       if (padapter->bDriverStopped) {
>
> 2.ii)
> +       if (pwrpriv->bInSuspend == true) {
> gets changed to:
> +       if (pwrpriv->bInSuspend) {
>
> 2.iii)
> +       if (pwrpriv->bInSuspend == false) {
> gets changed to:
> +       if (!pwrpriv->bInSuspend) {
>
> > Can you show all findings of this rule on the current rc or linux-next?
> >
>
> I have generated report on last 10k commits of latest next branch,
> where the check was found to be reported 5 times.
>
5 times in 10k commits? I am really wondering how you select the fix
options you work on?

Do you consider this relevant when the number is so low?
Are there potentially other classes that are much more relevant instead?

Can you run it over all files in the repository instead of git commits?

Commits really do not help when we want to fix this in the current kernel.


Lukas

> Here are the generated msgs:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/bool_comparison/last_10k.txt
>
> I have also generated the new CHK messages for BOOL_COMPARISON (over
> v4.13..v5.8) at:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/bool_comparison/commitsv4.13..v5.8.txt
>
> which might give us a better idea for what how the lines will be replaced.
>
> Thanks
> Aditya
>
> > Lukas
> >
> >> Suggested-by: Joe Perches <joe@perches.com>
> >> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> >> ---
> >> This fix is actually inspired from the comment mentioned in the script by Joe. I have added the Suggested-by line correspondingly.
> >>
> >>  scripts/checkpatch.pl | 7 +++++--
> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> >> index 8bd8e4518f7f..abd5a3d2e913 100755
> >> --- a/scripts/checkpatch.pl
> >> +++ b/scripts/checkpatch.pl
> >> @@ -6973,8 +6973,11 @@ sub process {
> >>                                         $op = "";
> >>                                 }
> >>
> >> -                               CHK("BOOL_COMPARISON",
> >> -                                   "Using comparison to $otype is error prone\n" . $herecurr);
> >> +                               if (CHK("BOOL_COMPARISON",
> >> +                                       "Using comparison to '$otype' is error prone. Perhaps use '${lead}${op}${arg}${trail}'\n" . $herecurr) &&
> >> +                                   $fix) {
> >> +                                       $fixed[$fixlinenr] =~ s/\b(?:true|false|$Lval)\s*(?:==|\!=)\s*(?:true|false|$Lval)\b/$op$arg/;
> >> +                               }
> >>
> >>  ## maybe suggesting a correct construct would better
> >>  ##                                 "Using comparison to $otype is error prone.  Perhaps use '${lead}${op}${arg}${trail}'\n" . $herecurr);
> >> --
> >> 2.17.1
> >>
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option and improve msg for BOOL_COMPARISON

[Aditya]

On 9/12/20 10:39 pm, Lukas Bulwahn wrote:
> On Wed, Dec 9, 2020 at 4:59 PM Aditya <yashsri421@gmail.com> wrote:
>>
>> On 9/12/20 5:17 pm, Lukas Bulwahn wrote:
>>> On Wed, Dec 9, 2020 at 11:58 AM Aditya Srivastava <yashsri421@gmail.com> wrote:
>>>>
>>>> Currently, checkpatch warns if the user compares expression with boolean.
>>>>
>>>> E.g., running checkpatch on commit 3b3a1a0b7824 ("staging: rtl8723bs:
>>>> hal: Modify comparison to constant in rtl8723bs_xmit.c") reports:
>>>>
>>>> CHECK: Using comparison to true is error prone
>>>> +       if (ret == true)
>>>>
>>>> Provide a fix by replacing the expression with preferred expression. i.e.,
>>>>
>>>> 1) If the check is for a 'true' or 'not false' expression, the operand
>>>> will get replaced with empty string.
>>>>
>>>> 2) Else, the operand gets replaced with '!', and the arguments follow.
>>>>
>>>
>>> Can you show the test cases you used to test this?
>>>
>> Yes, I ran it over certain commits.
>>
>> Eg 1) commit 5c789e131cbb ("netfilter: nf_conncount: Add list lock and
>> gc worker, and RCU for init tree search"), where
>>
>> 1.i)
>> +       if (list->dead == true) {
>> gets changed to:
>> +       if (list->dead) {
>>
>> 1.ii)
>> +               if (rbconn->list.count == 0 && rbconn->list.dead == false) {
>> gets changed to:
>> +               if (rbconn->list.count == 0 && !rbconn->list.dead) {
>>
>> E.g. 2) commit 83ee6ec7740b75dc0db (Staging: rtl8723bs: os_dep: Fix
>> if-else coding style issues) where
>>
>> 2.i)
>> +       if (padapter->bDriverStopped == true) {
>> gets changed to:
>> +       if (padapter->bDriverStopped) {
>>
>> 2.ii)
>> +       if (pwrpriv->bInSuspend == true) {
>> gets changed to:
>> +       if (pwrpriv->bInSuspend) {
>>
>> 2.iii)
>> +       if (pwrpriv->bInSuspend == false) {
>> gets changed to:
>> +       if (!pwrpriv->bInSuspend) {
>>
>>> Can you show all findings of this rule on the current rc or linux-next?
>>>
>>
>> I have generated report on last 10k commits of latest next branch,
>> where the check was found to be reported 5 times.
>>
> 5 times in 10k commits? I am really wondering how you select the fix
> options you work on?
> 
> Do you consider this relevant when the number is so low?
> Are there potentially other classes that are much more relevant instead?
> 
> Can you run it over all files in the repository instead of git commits?
> 
> Commits really do not help when we want to fix this in the current kernel.
> 

Hi
So, I ran  grep -e
"==\s*true\|==\s*false\|!=\s*true\|!=\s*false\|true\s*==\|false\s*==\|true\s*!=\|false\s*!="
. -R over the repo.
We currently have ~1630 such occurrences in different files.

Detail findings can be found here:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/bool_comparison/file_results.txt

A large amount of these occurrences are found in drivers/, with
"drivers/gpu/drm/amd/display/dc/dml/" having most occurrences.

What do you think?

Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option and improve msg for BOOL_COMPARISON

[Lukas Bulwahn]

On Sat, Dec 12, 2020 at 10:13 AM Aditya <yashsri421@gmail.com> wrote:
>
> On 9/12/20 10:39 pm, Lukas Bulwahn wrote:
> > On Wed, Dec 9, 2020 at 4:59 PM Aditya <yashsri421@gmail.com> wrote:
> >>
> >> On 9/12/20 5:17 pm, Lukas Bulwahn wrote:
> >>> On Wed, Dec 9, 2020 at 11:58 AM Aditya Srivastava <yashsri421@gmail.com> wrote:
> >>>>
> >>>> Currently, checkpatch warns if the user compares expression with boolean.
> >>>>
> >>>> E.g., running checkpatch on commit 3b3a1a0b7824 ("staging: rtl8723bs:
> >>>> hal: Modify comparison to constant in rtl8723bs_xmit.c") reports:
> >>>>
> >>>> CHECK: Using comparison to true is error prone
> >>>> +       if (ret == true)
> >>>>
> >>>> Provide a fix by replacing the expression with preferred expression. i.e.,
> >>>>
> >>>> 1) If the check is for a 'true' or 'not false' expression, the operand
> >>>> will get replaced with empty string.
> >>>>
> >>>> 2) Else, the operand gets replaced with '!', and the arguments follow.
> >>>>
> >>>
> >>> Can you show the test cases you used to test this?
> >>>
> >> Yes, I ran it over certain commits.
> >>
> >> Eg 1) commit 5c789e131cbb ("netfilter: nf_conncount: Add list lock and
> >> gc worker, and RCU for init tree search"), where
> >>
> >> 1.i)
> >> +       if (list->dead == true) {
> >> gets changed to:
> >> +       if (list->dead) {
> >>
> >> 1.ii)
> >> +               if (rbconn->list.count == 0 && rbconn->list.dead == false) {
> >> gets changed to:
> >> +               if (rbconn->list.count == 0 && !rbconn->list.dead) {
> >>
> >> E.g. 2) commit 83ee6ec7740b75dc0db (Staging: rtl8723bs: os_dep: Fix
> >> if-else coding style issues) where
> >>
> >> 2.i)
> >> +       if (padapter->bDriverStopped == true) {
> >> gets changed to:
> >> +       if (padapter->bDriverStopped) {
> >>
> >> 2.ii)
> >> +       if (pwrpriv->bInSuspend == true) {
> >> gets changed to:
> >> +       if (pwrpriv->bInSuspend) {
> >>
> >> 2.iii)
> >> +       if (pwrpriv->bInSuspend == false) {
> >> gets changed to:
> >> +       if (!pwrpriv->bInSuspend) {
> >>
> >>> Can you show all findings of this rule on the current rc or linux-next?
> >>>
> >>
> >> I have generated report on last 10k commits of latest next branch,
> >> where the check was found to be reported 5 times.
> >>
> > 5 times in 10k commits? I am really wondering how you select the fix
> > options you work on?
> >
> > Do you consider this relevant when the number is so low?
> > Are there potentially other classes that are much more relevant instead?
> >
> > Can you run it over all files in the repository instead of git commits?
> >
> > Commits really do not help when we want to fix this in the current kernel.
> >
>
> Hi
> So, I ran  grep -e
> "==\s*true\|==\s*false\|!=\s*true\|!=\s*false\|true\s*==\|false\s*==\|true\s*!=\|false\s*!="
> . -R over the repo.
> We currently have ~1630 such occurrences in different files.
>
> Detail findings can be found here:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/bool_comparison/file_results.txt
>
> A large amount of these occurrences are found in drivers/, with
> "drivers/gpu/drm/amd/display/dc/dml/" having most occurrences.
>
> What do you think?
>

I suggest you submit a few patches correcting this style to a few
driver maintainers to see if they are in favor of this checkpatch
rule.

Lukas

> Thanks
> Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option and improve msg for BOOL_COMPARISON

[Aditya]

On 13/12/20 1:33 pm, Lukas Bulwahn wrote:
> On Sat, Dec 12, 2020 at 10:13 AM Aditya <yashsri421@gmail.com> wrote:
>>
>> On 9/12/20 10:39 pm, Lukas Bulwahn wrote:
>>> On Wed, Dec 9, 2020 at 4:59 PM Aditya <yashsri421@gmail.com> wrote:
>>>>
>>>> On 9/12/20 5:17 pm, Lukas Bulwahn wrote:
>>>>> On Wed, Dec 9, 2020 at 11:58 AM Aditya Srivastava <yashsri421@gmail.com> wrote:
>>>>>>
>>>>>> Currently, checkpatch warns if the user compares expression with boolean.
>>>>>>
>>>>>> E.g., running checkpatch on commit 3b3a1a0b7824 ("staging: rtl8723bs:
>>>>>> hal: Modify comparison to constant in rtl8723bs_xmit.c") reports:
>>>>>>
>>>>>> CHECK: Using comparison to true is error prone
>>>>>> +       if (ret == true)
>>>>>>
>>>>>> Provide a fix by replacing the expression with preferred expression. i.e.,
>>>>>>
>>>>>> 1) If the check is for a 'true' or 'not false' expression, the operand
>>>>>> will get replaced with empty string.
>>>>>>
>>>>>> 2) Else, the operand gets replaced with '!', and the arguments follow.
>>>>>>
>>>>>
>>>>> Can you show the test cases you used to test this?
>>>>>
>>>> Yes, I ran it over certain commits.
>>>>
>>>> Eg 1) commit 5c789e131cbb ("netfilter: nf_conncount: Add list lock and
>>>> gc worker, and RCU for init tree search"), where
>>>>
>>>> 1.i)
>>>> +       if (list->dead == true) {
>>>> gets changed to:
>>>> +       if (list->dead) {
>>>>
>>>> 1.ii)
>>>> +               if (rbconn->list.count == 0 && rbconn->list.dead == false) {
>>>> gets changed to:
>>>> +               if (rbconn->list.count == 0 && !rbconn->list.dead) {
>>>>
>>>> E.g. 2) commit 83ee6ec7740b75dc0db (Staging: rtl8723bs: os_dep: Fix
>>>> if-else coding style issues) where
>>>>
>>>> 2.i)
>>>> +       if (padapter->bDriverStopped == true) {
>>>> gets changed to:
>>>> +       if (padapter->bDriverStopped) {
>>>>
>>>> 2.ii)
>>>> +       if (pwrpriv->bInSuspend == true) {
>>>> gets changed to:
>>>> +       if (pwrpriv->bInSuspend) {
>>>>
>>>> 2.iii)
>>>> +       if (pwrpriv->bInSuspend == false) {
>>>> gets changed to:
>>>> +       if (!pwrpriv->bInSuspend) {
>>>>
>>>>> Can you show all findings of this rule on the current rc or linux-next?
>>>>>
>>>>
>>>> I have generated report on last 10k commits of latest next branch,
>>>> where the check was found to be reported 5 times.
>>>>
>>> 5 times in 10k commits? I am really wondering how you select the fix
>>> options you work on?
>>>
>>> Do you consider this relevant when the number is so low?
>>> Are there potentially other classes that are much more relevant instead?
>>>
>>> Can you run it over all files in the repository instead of git commits?
>>>
>>> Commits really do not help when we want to fix this in the current kernel.
>>>
>>
>> Hi
>> So, I ran  grep -e
>> "==\s*true\|==\s*false\|!=\s*true\|!=\s*false\|true\s*==\|false\s*==\|true\s*!=\|false\s*!="
>> . -R over the repo.
>> We currently have ~1630 such occurrences in different files.
>>
>> Detail findings can be found here:
>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/bool_comparison/file_results.txt
>>
>> A large amount of these occurrences are found in drivers/, with
>> "drivers/gpu/drm/amd/display/dc/dml/" having most occurrences.
>>
>> What do you think?
>>
> 
> I suggest you submit a few patches correcting this style to a few
> driver maintainers to see if they are in favor of this checkpatch
> rule.
> 

Hi Lukas
I tried submitting the patches correcting this style, in various
subsystems as you suggested. Except staging/atomisp, every other
subsystem accepted the correction.

Do you think I should proceed with it?

Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees