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

[Lukas Bulwahn <lukas.bulwahn@gmail.com>]

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