* possible new false positive in checkpatch
@ 2015-09-12 12:13 Tal Shorer
2015-09-14 10:27 ` Andy Whitcroft
0 siblings, 1 reply; 6+ messages in thread
From: Tal Shorer @ 2015-09-12 12:13 UTC (permalink / raw)
To: tal.shorer, apw, joe; +Cc: linux-kernel
Since my last pull from upstream (today) , I started seeing some
checkpatch warnings regarding suspect code indent I believe are false
positive. Take this code for example:
static int foo(void)
{
while (bar())
/* do nothing */;
}
When running checkpath on it, the following warning is emitted:
tal@tal:~/Dev/lfs/linux|0 $ scripts/checkpatch.pl -f ~/tmp/foo.c
WARNING: suspect code indent for conditional statements (8, 32)
#3: FILE: /home/tal/tmp/foo.c:3:
+ while (bar())
+ /* do nothing */;
total: 0 errors, 1 warnings, 5 lines checked
/home/tal/tmp/foo.c has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
tal@tal:~/Dev/lfs/linux|1 $
Using my limited perl knowledge, I believe the lines causing this are
3111-3133:
# remove inline comments
$s =~ s/$;/ /g;
$c =~ s/$;/ /g;
Introduced in commit 9f5af480f4554aac12e002b6f5c2b04895857700:
checkpatch: improve SUSPECT_CODE_INDENT test
Commenting out these lines removes the warning.
This pattern exists in many places around the kernel source.
Is this the intended behavior?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: possible new false positive in checkpatch
2015-09-12 12:13 possible new false positive in checkpatch Tal Shorer
@ 2015-09-14 10:27 ` Andy Whitcroft
2015-09-15 14:50 ` Tal Shorer
0 siblings, 1 reply; 6+ messages in thread
From: Andy Whitcroft @ 2015-09-14 10:27 UTC (permalink / raw)
To: Tal Shorer; +Cc: joe, linux-kernel
On Sat, Sep 12, 2015 at 03:13:31PM +0300, Tal Shorer wrote:
> Since my last pull from upstream (today) , I started seeing some
> checkpatch warnings regarding suspect code indent I believe are false
> positive. Take this code for example:
>
> static int foo(void)
> {
> while (bar())
> /* do nothing */;
> }
>
> When running checkpath on it, the following warning is emitted:
>
> tal@tal:~/Dev/lfs/linux|0 $ scripts/checkpatch.pl -f ~/tmp/foo.c
> WARNING: suspect code indent for conditional statements (8, 32)
> #3: FILE: /home/tal/tmp/foo.c:3:
> + while (bar())
> + /* do nothing */;
>
> total: 0 errors, 1 warnings, 5 lines checked
>
> /home/tal/tmp/foo.c has style problems, please review.
>
> NOTE: If any of the errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
> tal@tal:~/Dev/lfs/linux|1 $
>
> Using my limited perl knowledge, I believe the lines causing this are
> 3111-3133:
> # remove inline comments
> $s =~ s/$;/ /g;
> $c =~ s/$;/ /g;
Yes it feels like that should be eliding them completely, and likely any
following space as well, something like this:
$s =~ s/$;+\s*//g;
$c =~ s/$;+\s*//g;
> Introduced in commit 9f5af480f4554aac12e002b6f5c2b04895857700:
> checkpatch: improve SUSPECT_CODE_INDENT test
> Commenting out these lines removes the warning.
>
> This pattern exists in many places around the kernel source.
> Is this the intended behavior?
Seems wrong to me.
-apw
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: possible new false positive in checkpatch
2015-09-14 10:27 ` Andy Whitcroft
@ 2015-09-15 14:50 ` Tal Shorer
2015-09-15 15:01 ` Andy Whitcroft
0 siblings, 1 reply; 6+ messages in thread
From: Tal Shorer @ 2015-09-15 14:50 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: joe, <linux-kernel@vger.kernel.org>
On Mon, Sep 14, 2015 at 1:27 PM, Andy Whitcroft <apw@canonical.com> wrote:
>
> On Sat, Sep 12, 2015 at 03:13:31PM +0300, Tal Shorer wrote:
> > Since my last pull from upstream (today) , I started seeing some
> > checkpatch warnings regarding suspect code indent I believe are false
> > positive. Take this code for example:
> >
> > static int foo(void)
> > {
> > while (bar())
> > /* do nothing */;
> > }
> >
> > When running checkpath on it, the following warning is emitted:
> >
> > tal@tal:~/Dev/lfs/linux|0 $ scripts/checkpatch.pl -f ~/tmp/foo.c
> > WARNING: suspect code indent for conditional statements (8, 32)
> > #3: FILE: /home/tal/tmp/foo.c:3:
> > + while (bar())
> > + /* do nothing */;
> >
> > total: 0 errors, 1 warnings, 5 lines checked
> >
> > /home/tal/tmp/foo.c has style problems, please review.
> >
> > NOTE: If any of the errors are false positives, please report
> > them to the maintainer, see CHECKPATCH in MAINTAINERS.
> > tal@tal:~/Dev/lfs/linux|1 $
> >
> > Using my limited perl knowledge, I believe the lines causing this are
> > 3111-3133:
> > # remove inline comments
> > $s =~ s/$;/ /g;
> > $c =~ s/$;/ /g;
>
> Yes it feels like that should be eliding them completely, and likely any
> following space as well, something like this:
>
> $s =~ s/$;+\s*//g;
> $c =~ s/$;+\s*//g;
>
Replacing the problematic lines with these fixes the issue.
> > Introduced in commit 9f5af480f4554aac12e002b6f5c2b04895857700:
> > checkpatch: improve SUSPECT_CODE_INDENT test
> > Commenting out these lines removes the warning.
> >
> > This pattern exists in many places around the kernel source.
> > Is this the intended behavior?
>
> Seems wrong to me.
>
> -apw
Which git tree is checkpatch developed in? Linus's?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: possible new false positive in checkpatch
2015-09-15 14:50 ` Tal Shorer
@ 2015-09-15 15:01 ` Andy Whitcroft
2015-10-26 16:43 ` Tal Shorer
2016-06-11 7:33 ` Tal Shorer
0 siblings, 2 replies; 6+ messages in thread
From: Andy Whitcroft @ 2015-09-15 15:01 UTC (permalink / raw)
To: Tal Shorer; +Cc: joe, <linux-kernel@vger.kernel.org>
On Tue, Sep 15, 2015 at 05:50:40PM +0300, Tal Shorer wrote:
> > Yes it feels like that should be eliding them completely, and likely any
> > following space as well, something like this:
> >
> > $s =~ s/$;+\s*//g;
> > $c =~ s/$;+\s*//g;
> >
> Replacing the problematic lines with these fixes the issue.
> > > Introduced in commit 9f5af480f4554aac12e002b6f5c2b04895857700:
> > > checkpatch: improve SUSPECT_CODE_INDENT test
> > > Commenting out these lines removes the warning.
> > >
> > > This pattern exists in many places around the kernel source.
> > > Is this the intended behavior?
> >
> > Seems wrong to me.
> >
> > -apw
>
> Which git tree is checkpatch developed in? Linus's?
Yeah in Linus' tree.
-apw
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: possible new false positive in checkpatch
2015-09-15 15:01 ` Andy Whitcroft
@ 2015-10-26 16:43 ` Tal Shorer
2016-06-11 7:33 ` Tal Shorer
1 sibling, 0 replies; 6+ messages in thread
From: Tal Shorer @ 2015-10-26 16:43 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: joe, <linux-kernel@vger.kernel.org>
ping?
Is there a plan to fix it? should I send your fix? (kinda plagiarism)
next release? (which would mean the unnecessary warnings will exist in 4.3 :<)
On Tue, Sep 15, 2015 at 6:01 PM, Andy Whitcroft <apw@canonical.com> wrote:
> On Tue, Sep 15, 2015 at 05:50:40PM +0300, Tal Shorer wrote:
>
>> > Yes it feels like that should be eliding them completely, and likely any
>> > following space as well, something like this:
>> >
>> > $s =~ s/$;+\s*//g;
>> > $c =~ s/$;+\s*//g;
>> >
>> Replacing the problematic lines with these fixes the issue.
>> > > Introduced in commit 9f5af480f4554aac12e002b6f5c2b04895857700:
>> > > checkpatch: improve SUSPECT_CODE_INDENT test
>> > > Commenting out these lines removes the warning.
>> > >
>> > > This pattern exists in many places around the kernel source.
>> > > Is this the intended behavior?
>> >
>> > Seems wrong to me.
>> >
>> > -apw
>>
>> Which git tree is checkpatch developed in? Linus's?
>
> Yeah in Linus' tree.
>
> -apw
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: possible new false positive in checkpatch
2015-09-15 15:01 ` Andy Whitcroft
2015-10-26 16:43 ` Tal Shorer
@ 2016-06-11 7:33 ` Tal Shorer
1 sibling, 0 replies; 6+ messages in thread
From: Tal Shorer @ 2016-06-11 7:33 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: joe, <linux-kernel@vger.kernel.org>
On Tue, Sep 15, 2015 at 6:01 PM, Andy Whitcroft <apw@canonical.com> wrote:
> On Tue, Sep 15, 2015 at 05:50:40PM +0300, Tal Shorer wrote:
>
>> > Yes it feels like that should be eliding them completely, and likely any
>> > following space as well, something like this:
>> >
>> > $s =~ s/$;+\s*//g;
>> > $c =~ s/$;+\s*//g;
>> >
>> Replacing the problematic lines with these fixes the issue.
>> > > Introduced in commit 9f5af480f4554aac12e002b6f5c2b04895857700:
>> > > checkpatch: improve SUSPECT_CODE_INDENT test
>> > > Commenting out these lines removes the warning.
>> > >
>> > > This pattern exists in many places around the kernel source.
>> > > Is this the intended behavior?
>> >
>> > Seems wrong to me.
>> >
>> > -apw
>>
>> Which git tree is checkpatch developed in? Linus's?
>
> Yeah in Linus' tree.
>
> -apw
This still isn't fixed. Should I submit Andy's change myself? Feels
like plagiarize.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-11 7:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-12 12:13 possible new false positive in checkpatch Tal Shorer
2015-09-14 10:27 ` Andy Whitcroft
2015-09-15 14:50 ` Tal Shorer
2015-09-15 15:01 ` Andy Whitcroft
2015-10-26 16:43 ` Tal Shorer
2016-06-11 7:33 ` Tal Shorer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).