linux-kernel - Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN

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: <20120314123429.GG15379@thunk.org>
Date:	Wed, 14 Mar 2012 08:34:29 -0400
From:	Ted Ts'o <tytso@....edu>
To:	Joe Perches <joe@...ches.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Andy Whitcroft <apw@...dowen.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

On Tue, Mar 13, 2012 at 08:01:14PM -0700, Joe Perches wrote:
> > That's a debug message which is never by anyone other than ext4
> > developers.  Your patch also hacked the Makefile to enable it by
> > default,
> 
> It's just an example and no it didn't.
> That output is still in an #ifdef EXT4FS_DEBUG
> block and is unchanged.

I looked at your patch, and nearly all of them were in debug code.  I
know, because in practice the messages that come up with any kind of
regularity are all properly prefixed.

Look, the reason why I care about patch noise is because I have a huge
patch backlog.  Take a look at this:

      http://patchwork.ozlabs.org/project/linux-ext4/list/

Very few other people review patches, and even patches that survive
review, I've found problems that could potentially lead to data loss
or system instability.  This is not like your average device driver,
where if the machine panics once a week, "oh well", and you reboot.
Linus would get very cranky if he lost data as a result of a bad patch
slipping through.  Hence, patches don't go in until after significant
review and testing.

As a result, #1, patches that are don't add value, and are large,
simply won't get applied.  Period.  Avoiding the downside of lots of
people losing data is ****far**** more than your OCD wanting me to use
pr_warn(...) instead of printk(KERN_WARN, ...).

If you want your style patches to go in, break them into smaller
chunks, or I *will* ignore them.

#2, patches that don't add substantial value, and make it harder for
me to review and apply patches from my very substantial patch
backlog, I consider as adding ***substantial*** negative value.

That being said, I use checkpatch.pl as an initial screen, but it's
already been the case that there are warnings that I consider pure
noise, and I really, REALLY, rather not add more noise to
checkpatch.pl.  There is no group consensus about things like pr_foo
--- not as far as I've seen --- and to impose it from on high by some
patch wankers making changes to checkpatch.pl very much offends me.

      	      	     	     		      	   - Ted
--
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/