linux-ext4 - Re: [PATCH 01/10] Fs: ext4: acl.c: fixed indent issue

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: <20100926010410.GC19690@thunk.org>
Date:	Sat, 25 Sep 2010 21:04:11 -0400
From:	Ted Ts'o <tytso@....edu>
To:	Joe Perches <joe@...ches.com>
Cc:	Christoph Hellwig <hch@....de>, T Dent <tdent48227@...il.com>,
	adilger.kernel@...ger.ca, jack@...e.cz, dmonakhov@...nvz.org,
	sandeen@...hat.com, linux-ext4@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/10] Fs: ext4: acl.c: fixed indent issue

On Sat, Sep 25, 2010 at 05:50:43PM -0700, Joe Perches wrote:
> The reason get_maintainers by default cc'd signers is mostly
> historical.  The file pattern coverage in MAINTAINERS when
> it was added wasn't very good, so signers were always added.
> It was also the Linus' preferred mechanism to find those
> "who really do the work".
> 
> http://lkml.org/lkml/2007/8/14/276

Yeah, but Linus said "subdirectory or file".  He was also assuming
some human intelligence.  In the case of a filesystem, you should use
who has done most of the work in the *subdirectory*, and not on a
file-by-file basis.  The brokeness is trying to do this in blind and
stupid script, that can't understand the difference between when you
should use subdirectory or a file.

This is what caused the completely insane result for fs/ext4/acl.c.
There have only been three commits in the past year, and they have all
been random folks who were doing random fixups --- many of which might
be stylistic cleanups.  That's not "the people who really do the
work".  It's just "the people who happened to do some random cleanups
on a file that happens not to change much".  But the stupid thing is
trying to do it on a file-by-file basis in the first place, when for
something like fs/ext4, it really should be done on a subdirectory
basis.

That, BTW, is also my biggest complaint about checkpatch.pl.  If it's
used by newbies who want to get warned about obvious things, that's
fine.  If it's used by maintainers as an automated way to catch nits,
that's also fine.  Maintainers are experts who know when it's OK to
disregard flase positives.

What really annoys me is newbies who use checkpatch.pl in its --file
mode, and then assume that every single warning is a deadly bug that
much be patched.  Scripts by definitions are stupid, and don't
substitute for thinking.  checkpatch.pl at least as the excuse that it
has some valid non-stupid uses.  But I'm not convinced
get_maintainers.pl has the same excuse.  

I at least never use it.  I'll look through the MAINTAINERS file by
hand, or I'll use git log by hand, and let my human intelligence
figure out whether or not the patches that are turned up constitute
"those that do real work", or are bullshit checkpatch.pl cleanup
patches.  Training people to use a script that by defintion can't be
smart enough to make these distinction ultimately is a huge disservice
to newbies (and experts won't use get_maintinaer.pl anyway, because
they will want to know the context).

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html