On 02.02.2015 15:16, Al Viro wrote: ... and a very nice day to you, too, sir! 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. Regards, Andreas

On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote: 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.

On Thu, 2015-02-05 at 20:25 +0300, Dan Carpenter wrote: Maybe I should add sparse too... ;)

On Thu, Feb 05, 2015 at 09:32:55AM -0800, Joe Perches wrote: No need. Smatch users are awesome at writing changelogs. regards, dan carpenter

On Thu, 2015-02-05 at 20:35 +0300, Dan Carpenter wrote: :) good point, kinda... $ git log --format=oneline | grep -wi smatch | wc -l 141 $ git log --pretty=oneline | grep -wi sparse | wc -l 2383 $ git log --pretty=oneline | grep -Pi "\bcheckpatch\b[^:]" | wc -l 1637

On Thu, Feb 5, 2015 at 5:57 PM, Greg Kroah-Hartman <gregkh(a)linuxfoundation.org&gt; wrote: Sorry for that. I agree a proper a description is needed ideally, but all these days prior to my patches, the subject line was 'fix sparse warnings' for such patches, that’s the reason I picked it for my script. I understand people get annoyed seeing so many continuous patches with same subject line, ill make my script a bit smarter to have detailed explanation now on. Just a side note I verify the patch created by the script and if its OK then only I post it to ML. Regards, --Prabhakar Lad

On Thu, 2015-02-05 at 09:57 -0800, Greg Kroah-Hartman wrote: Mind if I ask which person/bot you have in mind? People are trying to bot away the kconfig related messages I send out - which I think is a very good idea - and I'd like to know what level that bot should reach before people can get out of its way. (I happen to think that the stuff I currently do is best done with a human filter applied.) Thanks, Paul Bolle

On Thu, 2015-02-05 at 12:06 -0800, Greg Kroah-Hartman wrote: Hmm. That proves bots should be used to create patches.