2023-12-22 23:16:16

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: ignore deleted lines for comment context

On Fri, 2023-12-22 at 16:05 -0700, Andreas Dilger wrote:
> From: Andreas Dilger <[email protected]>
>
> Don't consider lines being removed by a patch as part of a comment.
> Otherwise, false "WARNING: memory barrier without comment" and similar
> issues can be reported when a comment does exist on the previous line.
>
> For example, a change like below was previously incorrectly flagged:
>
> /* matched by smp_store_release() in some_function() */
> - if (smp_load_acquire(&list->tail) == head))
> + if (smp_load_acquire(&list->tail) == head) && flags == 0)
>
> Signed-off-by: Andreas Dilger <[email protected]>

OK, but:

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -1972,7 +1972,8 @@ sub ctx_locate_comment {
> $current_comment = '';
> for (my $linenr = $first_line; $linenr < $end_line; $linenr++) {
> my $line = $rawlines[$linenr - 1];
> - #warn " $line\n";
> + #warn "LINE($linenr): $line\n";

This is a superfluous change

> + next if ($line =~ /^-/); # ignore lines removed by patch
> if ($linenr == $first_line and $line =~ m@^.\s*\*@) {
> $in_comment = 1;
> }



2023-12-22 23:22:42

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: ignore deleted lines for comment context

On Dec 22, 2023, at 4:15 PM, Joe Perches <[email protected]> wrote:
>
> On Fri, 2023-12-22 at 16:05 -0700, Andreas Dilger wrote:
>> From: Andreas Dilger <[email protected]>
>>
>> Don't consider lines being removed by a patch as part of a comment.
>> Otherwise, false "WARNING: memory barrier without comment" and similar
>> issues can be reported when a comment does exist on the previous line.
>>
>> For example, a change like below was previously incorrectly flagged:
>>
>> /* matched by smp_store_release() in some_function() */
>> - if (smp_load_acquire(&list->tail) == head))
>> + if (smp_load_acquire(&list->tail) == head) && flags == 0)
>>
>> Signed-off-by: Andreas Dilger <[email protected]>
>
> OK, but:
>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
>> @@ -1972,7 +1972,8 @@ sub ctx_locate_comment {
>> $current_comment = '';
>> for (my $linenr = $first_line; $linenr < $end_line; $linenr++) {
>> my $line = $rawlines[$linenr - 1];
>> - #warn " $line\n";
>> + #warn "LINE($linenr): $line\n";
>
> This is a superfluous change

Sure, I can resubmit without it if you want. I found it helpful when
debugging why this code didn't work in the first place...

>
>> + next if ($line =~ /^-/); # ignore lines removed by patch
>> if ($linenr == $first_line and $line =~ m@^.\s*\*@) {
>> $in_comment = 1;
>> }
>


Cheers, Andreas






Attachments:
signature.asc (890.00 B)
Message signed with OpenPGP