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. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
