Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1185715 > unrolled thread

Checkpatch: False positive

Back to article view | Back to linux.kernel

Contents

  Checkpatch: False positive Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-16 13:00 +0200
    Re: Checkpatch: False positive Joe Perches <joe@perches.com> - 2015-07-16 17:40 +0200
      Re: Checkpatch: False positive Andy Whitcroft <apw@canonical.com> - 2015-07-16 17:50 +0200
        Re: Checkpatch: False positive Joe Perches <joe@perches.com> - 2015-07-16 18:00 +0200
          Re: Checkpatch: False positive Andy Whitcroft <apw@canonical.com> - 2015-07-16 19:30 +0200

#1185715 — Checkpatch: False positive

Hi Andy/Joe,

I got a warning today for my cover-letter, and it looked like a false
positive. Please have a look, based of v4.2-rc2.

-----------------------
0000-cover-letter.patch
-----------------------
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#31: 
 arch/x86/kernel/hpet.c         | 198 ++++++++++++++++++++++++++---------------

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1185981

On Thu, 2015-07-16 at 16:25 +0530, Viresh Kumar wrote:
> Hi Andy/Joe,
> 
> I got a warning today for my cover-letter, and it looked like a false
> positive. Please have a look, based of v4.2-rc2.
> -----------------------
> 0000-cover-letter.patch
> -----------------------
> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> #31: 
>  arch/x86/kernel/hpet.c         | 198 ++++++++++++++++++++++++++---------------

There are a lot of other false positives for this test.

Maybe it's not worth checking for this condition and the
test should be removed as "fixing" it may not be feasible.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186010

On Thu, Jul 16, 2015 at 08:35:58AM -0700, Joe Perches wrote:
> > #31: 
> >  arch/x86/kernel/hpet.c         | 198 ++++++++++++++++++++++++++---------------

I guess those are in the limbo land between the end of message and
beginning of the patch itself.  Perhaps the test should at least stop at
the end of header marker, at the '---'.

-apw
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186021

On Thu, 2015-07-16 at 16:43 +0100, Andy Whitcroft wrote:
> On Thu, Jul 16, 2015 at 08:35:58AM -0700, Joe Perches wrote:
> > > #31: 
> > >  arch/x86/kernel/hpet.c         | 198 ++++++++++++++++++++++++++---------------
> 
> I guess those are in the limbo land between the end of message and
> beginning of the patch itself.  Perhaps the test should at least stop at
> the end of header marker, at the '---'.
> 
> -apw

Maybe, but the test already stops at signatures like
Signed-off-by: that should always be above the ---.

This might help, but there are _many_ false positives.

The other thing that might help is for people to take
the warnings the script produces less seriously.

Maybe convert:

ERROR -> defect
WARNING -> unstylish
CHECK -> nitpick

or some such

---

 scripts/checkpatch.pl | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d5ce29a..5e7afa7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2213,6 +2213,11 @@ sub process {
 			$in_commit_log = 0;
 		}
 
+# Check for patch separator
+		if ($line =~ /^---$/) {
+			$in_commit_log = 0;
+		}
+
 # Check if MAINTAINERS is being updated.  If so, there's probably no need to
 # emit the "does MAINTAINERS need updating?" message on file add/move/delete
 		if ($line =~ /^\s*MAINTAINERS\s*\|/) {


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186088

On Thu, Jul 16, 2015 at 08:58:56AM -0700, Joe Perches wrote:
> On Thu, 2015-07-16 at 16:43 +0100, Andy Whitcroft wrote:
> > On Thu, Jul 16, 2015 at 08:35:58AM -0700, Joe Perches wrote:
> > > > #31: 
> > > >  arch/x86/kernel/hpet.c         | 198 ++++++++++++++++++++++++++---------------
> > 
> > I guess those are in the limbo land between the end of message and
> > beginning of the patch itself.  Perhaps the test should at least stop at
> > the end of header marker, at the '---'.
> > 
> > -apw
> 
> Maybe, but the test already stops at signatures like
> Signed-off-by: that should always be above the ---.
> 
> This might help, but there are _many_ false positives.
> 
> The other thing that might help is for people to take
> the warnings the script produces less seriously.
> 
> Maybe convert:
> 
> ERROR -> defect
> WARNING -> unstylish
> CHECK -> nitpick

Heh, that has long been the main issue, please please believe your brain
not checkpatch.  But yes some less inflamitory words might, just might,
reduce the noise.

-apw
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web