linux-kernel - Re: [PATCH] Deprecate checkpatch.pl --file

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: <200801082127.21602.bzolnier@gmail.com>
Date:	Tue, 8 Jan 2008 21:27:21 +0100
From:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To:	Sam Ravnborg <sam@...nborg.org>
Cc:	Daniel Walker <dwalker@...sta.com>,
	Andi Kleen <andi@...stfloor.org>, linux-kernel@...r.kernel.org,
	apw@...dowen.org, akpm@...ux-foundation.org
Subject: Re: [PATCH] Deprecate checkpatch.pl --file

On Tuesday 08 January 2008, Sam Ravnborg wrote:
> On Tue, Jan 08, 2008 at 08:50:33AM -0800, Daniel Walker wrote:
> > 
> > On Tue, 2008-01-08 at 17:14 +0100, Andi Kleen wrote:
> > 
> > 
> > > +if ($file) {
> > > +	print <<EOL
> > > +WARNING: Using --file mode. Please do not send patches to linux-kernel
> > > +that change whole existing files if you did not significantly change most
> > > +of the the file for other reasons anyways or just wrote the file newly
> > > +from scratch. Pure code style patches have a significant cost in a
> > > +quickly changing code base like Linux because they cause rejects
> > > +with other changes.
> > > +If you're sure you want to use whole file mode please use --file-force
> > > +EOL
> > > +;
> > > +	exit(1);

I'm not against the patch (although --file to --file-force parameter change
seems unnecessary and the warning should be toned down) but I think that
cleanup patches have been demonized out of proportion recently on LKML.

> > Can't say I like this too much .. It sounds like your telling people to
> > stop sending cleanup patches, which doesn't make much sense .. We want a
> > clean kernel .. Or at least I do ..

Seconded, also if this is what people like to work on we shouldn't be trying
to stop them but point them into the direction where their work could be more
productive in "the big picture" context.

> It is a simple pain/benefit issue.
> Fixing the 25 errors and 13 warnings in kernel/profile.c may look
> like an easy task but then we put additional burden on the 10 people
> that have patches pending for this file.

OTOH there is a lot of old/legacy code that almost nobody is touching where
cleanup would be appreciated (it is quite likely that somebody will notice
and fix a bug or two in the process).

> It is the same reason why we do not run a 'kill-trailing-whitespace'
> on the full kernel tree.

Agreed but we sometimes accept per file whitespace cleanups.

The important thing here is "common sense".  Sending a big patch series
of "checkpatch.pl fixes" for kernel/* files is certainly not a good thing.
Core code has a lot of high-profile people looking over it so you will be
just interfering with their changes.

However if somebody do such fixes for some old/orphaned driver _and_ also
fix other things at the same time then everything is sweet from my POV. :)

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