Re: [PATCH] Change judgment len position

[Al Viro <viro@ZenIV.linux.org.uk>]

On Wed, Oct 24, 2018 at 06:16:31PM -0700, Joe Perches wrote:
> On Thu, 2018-10-25 at 09:11 +0800, Fengguang Wu wrote:
> > CC Philip and LKP team.
> > > 	Please try to make your first patch in drivers/staging
> > > 	to get familiar with submitting patches to the kernel.
> > > 	https://kernelnewbies.org/FirstKernelPatch
> > > 
> > > Maybe there's utility in creating a filtering and auto-reply
> > > tool for first time patch submitters for all the vger mailing
> > > lists using some combination of previously known submitters
> > > and the 0-day robot to point those first time submitters of
> > > defective patches to kernelnewbies and staging.
> > 
> > Yeah good idea. That feature can be broken into 2 parts:
> > 
> > - an email script, which could be added to Linux scripts/ dir 
> > - maintain records for telling whether someone is first-time patch submitters
> 
> Maybe run checkpatch on those first-time submitter patches too.

OK, now I'm certain that you are trolling...

Joe, what really pisses me off is that it's actually at the expense of original
poster (who had nothing to do with the CoCup) *and* an invitation for a certain
variety of kooks.  In probably vain hope of heading that off, here's the
summary of what happened _before_ Joe started to stir the shit:

	* code in question is, indeed, (slightly) bogus in mainline.
It reads as "reject negative values for length, truncate positive ones to 4",
but in reality it's "treat everything outside of 0..4 as 4".  It's not broken
per se, but it's certainly misleading.
	* one possible fix would be to drop the "reject negative values"
completely, another - to move checking for negatives before the truncation.
Patch tried to do the latter.
	* the author of the patch has moved the check *too* early -
before the value being tested is even obtained.	It's obviously wrong - kernel
newbie or not.
	* I sincerely doubt that it was an attempt to introduce a backdoor,
albeit one would've been created if that patch went in as-is.  Genuine braino
is much more likely, and we'd all done such.
	* such brainos can be surprisingly hard to spot in one's code.
It's too obviously wrong, in a sense - you know what you've meant to write
and you keep seeing that instead of what you've actually written.  If you
are really unlucky, that might end up with a few days worth of debugging,
with eventual embarrassed "how the fuck have I managed not to notice that
previous twenty times I went over this function today?"  Been there,
done that, and so has everyone who'd actually written anything (other
than the worthless screeds, that is).
	* one thing the author of that patch could be blamed for (and most
of us have fucked up that way at one point or another) is not testing the
effect of the damn thing.  Modification is local, the change of behaviour -
simple and triggering that code is also trivial.  Checking that the patch
has done what you expect it to do would be simple and would've caught the
problem.
`
	Code of Conduct is garbage, but neither Dave nor the author
of this patch had anything to do with that mess.  If you want to make a point,
do so without shit splatter hitting innocent bystanders - people tend to
get very annoyed by that kind of thing, and with a damn good reason.
Sheesh...