Message-ID: <20150205163053.GQ29656@ZenIV.linux.org.uk>
Date:	Thu, 5 Feb 2015 16:30:53 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Andreas Ruprecht <rupran@...server.de>
Cc:	Oleg Drokin <oleg.drokin@...el.com>,
	Andreas Dilger <andreas.dilger@...el.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	HPDD-discuss@...1.01.org, devel@...verdev.osuosl.org,
	linux-kernel@...r.kernel.org,
	"Lad, Prabhakar" <prabhakar.csengg@...il.com>
Subject: use of opaque subject lines

On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote:

> On a serious note: I do understand what you're getting at, I don't take
> that personally (and I will send a v2 addressing the things above), but
> honestly, this kind of answer might just be a real turn-off for other
> people trying to get into kernel development...
> 
> I don't want to start a whole new 'attitude in the kernel community'
> discussion, but I can't just let this go like that, sorry.

Just during the last 12 hours or so, I've seen the following l-k traffic:

Subject: [PATCH] usb: host/sl811-hcd: fix sparse warning
Subject: [PATCH] usb: gadget: function/f_sourcesink: fix sparse warning
Subject: [PATCH] tty: vt/vt: fix sparse warning
Subject: [PATCH] scsi: fix sparse warnings
Subject: [PATCH] bfa: bfa_core: fix sparse warning
Subject: [PATCH] scsi: fix sparse warning
Subject: [PATCH] xen/acpi-processor: fix sparse warning
Subject: [PATCH] scsi: initio: fix sparse warnings
Subject: [PATCH] scsi: dc395x: fix sparse warning
Subject: [PATCH] scsi: eata: fix sparse warning
Subject: [PATCH] scsi: qla1280: fix sparse warnings
Subject: [PATCH] scsi: ips: fix sparse warnings
Subject: [PATCH] fbdev: via/via_clock: fix sparse warning
Subject: [PATCH] usb: gadget: fix sparse warnings
Subject: [PATCH] usb: gadget: fix sparse warnings
Subject: [PATCH] usb: gadget: function/uvc_v4l2.c: fix sparse warnings
Subject: [PATCH] xen-netback: fix sparse warning
Subject: [PATCH] thermal: int340x: fix sparse warning
Subject: [PATCH] vxge: fix sparse warning
Subject: Re: [PATCH] xen-netback: fix sparse warning
Subject: [PATCH] ixgbe: fix sparse warnings
Subject: [PATCH] samsung-laptop: fix sparse warning
Subject: [PATCH] x86: thinkpad_acpi.c: fix sparse warning
Subject: [PATCH] Sony-laptop: fix sparse warning

one of those is an ACK from maintainer, the rest are (AFAICS) all "make
something static", all with rather poor commit messages and subjects.  Not
threaded, either (that would've somewhat mitigated one of the problems).

Bad attitude?  I'd say that _this_ is one.  l-k is high-traffic list;
postings like that mean extra strain on those who are trying at least to
skim through it and they are not easy on those who are trying to read the
commit logs.  And it's trivial to mitigate - choose less opaque subjects,
for starters.  Because these are about as opaque as it gets: essentially,
it's "sparse had drawn my attention to $FILE; I've fixed that problem by
making it STFU".  Readers would like more information to filter upon?
Tough luck, open the damn mail and look into it.  One after another, after
another, to the tune of dozens per night.  All with the commit messages
pretty much of the form "$TOOL has produced $MESSAGE; with diff below it
doesn't".

It's physically impossible to read through all l-k traffic; the rate is too
high for that.  Is making it possible to do minimal triage too much to ask for?
I'm not blaming anyone for going after the low-hanging fruit like that to
get some experience on simple stuff, but as it is, it reinforces seriously
bad habits while creating considerable PITA for list readers.  And yes,
we'd all done our share of lousy commit messages, but this pretty much
teaches newbies to do that all the time.

Sigh...  Folks, please
	* Use descriptive subject lines at least somewhere in a thread.
	* Describe what the thing does, besides "makes $TOOL to STFU".
	* Remember, you don't fix warnings - you deal with the problem
(if any) in the code those warnings had pointed to.  And if there isn't
a problem (i.e. the warning was a false positive) you might be annotating
the code to tell the dumb tool what's going on there.  Which is a different
genre, and needs different mindset when reviewing (it's easy to obfuscate
a _real_ problem away - away from the $TOOL's ability to spot it, that
is).
	* Remember that said tools are nowhere near strong AI; the
same warning can come from very different underlying problems.  These
warnigns are heuristics - such-and-such looking code has high chances of
needing attention.  Give at least some indication what the problem _is_ - e.g.
the sparse warnings that has spawned this flood might come from
	1) function really is used only in the file it's defined in,
never intended to be used elsewhere, ought to be static to avoid namespace
pollution
	2) function is used elsewhere, and its uses elsewhere have explicit
externs right at the point of use.  Dangerous, since there's no practical
way for compiler (gcc, sparse, lint, whatever) to verify that there's no
type mismatch.  Ought to have extern placed in some header included both by
the file where it's defined and by all files where it's used, with externs
at the point of use removed.  Need to be careful about the choice of header
and location in it.
	3) function _is_ declared in a common header, but it's either not
included by the file where it actually lives, *or* is included only on
some architectures and/or configs.  Ought to figure out which one it is,
and either add an include or (needs _much_ more care) make some of
the includes in the chain of indirect includes unconditional.
	4) function is declared in a common header, and it is included,
but declaration is under the too tight set of ifdefs.  Might need to move
it around, but that also requires some care - less than what you need when
messing with conditional indirect includes, but also a non-trivial amount.
	Note that "I've made it static and result builds, so it must be
(1)" is incorrect - the code elsewhere actually using it might have been
ifdefed out on your config and architecture, simply not included into your
build, or optimized away (again, on your config and architecture).  "This
identifier is never mentioned elsewhere" is better (albeit still not
fool-proof - creative use of ## in macros might end up with something like
FOO_FUNC(add3) hiding a use of silly_prefix_add3_you_wont_find_me_with_grep,
and yes, things like that do happen).
	* When mass-submitting, use threading.  It's much easier on maillist
readers.
	* When using threading, at least try to keep the patches in one
thread related to each other.
--
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/

Powered by blists - more mailing lists