linux-kernel - Re: [PATCH] checkpatch: do not test/warn of leading whitespace before signature tags

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1313655414.2128.102.camel@jtkirshe-mobl>
Date:	Thu, 18 Aug 2011 01:16:53 -0700
From:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
To:	Joe Perches <joe@...ches.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Allan, Bruce W" <bruce.w.allan@...el.com>,
	Anish Kumar <anish198519851985@...il.com>,
	Andy Whitcroft <apw@...onical.com>
Subject: Re: [PATCH] checkpatch: do not test/warn of leading whitespace
 before signature tags

On Thu, 2011-08-18 at 00:59 -0700, Joe Perches wrote:
> On Thu, 2011-08-18 at 00:47 -0700, Jeff Kirsher wrote:
> > On Thu, 2011-08-18 at 00:26 -0700, Joe Perches wrote:
> > > On Wed, 2011-08-17 at 23:48 -0700, Jeff Kirsher wrote:
> > > > From: Bruce Allan <bruce.w.allan@...el.com>
> > > > 
> > > > Commit 2011247 introduced additional style checks for signature tags in
> > > > patches which is good.  Unfortunately, now whenever patches are checked
> > > > by piping the output of 'git show' or 'stg show' through checkpatch it
> > > > warns not to use whitespace before all signature tags since these (and the
> > > > rest of the patch description) are indented.  Remove this test/warning.
> > > 
> > > I think this is not a good idea.
> > > 
> > > checkpatch is meant for patches not git log output.
> > > indenting signatures can cause other problems later.
> > > 
> > > I think you can avoid this easily by using checkpatch
> > > option --ignore=BAD_SIGN_OFF when using git log output
> > > as input.
> > 
> > The problem I have with this is that the sign-off's are not bad, they
> > are by default indented by 'git show' or 'stg show' so checkpatch.pl
> > should handle the "default" formatting of git/stg and if there is
> > additional indenting not expected, then the sign-off's should be
> > considered bad.
> 
> I disagree.
> 
> checkpatch should handle the default input of patches
> as best it can.
> 
> I suppose checkpatch could have a different "--input=git"
> or some such to avoid certain things that git might produce
> that a patch would not.
> 
> Deleting useful checks for patches isn't a good idea.
> 
> > If this option is added, then if there were "real"
> > problems with the sign-off, it would not be displayed.
> 
> So what?
> 
> It would also be too late to do anything about
> it anyway as it would already be committed.

To add...

Specifically, most maintainers ask developers to run checkpatch.pl on
their patches before submitting them to the community.  As part of our
patch validation process, our validation team runs checkpatch.pl on
every patch I submit and we are now getting constant warnings about good
sign-off's with this recent change.  I personally end up reviewing all
the warnings to ensure that they are false warnings and not "real"
issues with the sign-off's.

> 
> > > You could also use: 
> > > git log --format="commit %H%nAuthor: %an <%ae>%nDate:   %aD%n%n%s%n%n%b"
> > > so that you get the current default --format=medium
> > > output without indenting the commit log body.
> > Even doing this does not resolve the "false" warnings" that
> > checkpatch.pl produces regarding the sign-off's.
> 
> I tried it.  It works for me.
> What about it doesn't work for you?
> 
> 



Download attachment "signature.asc" of type "application/pgp-signature" (491 bytes)