On Tue, 2016-02-16 at 12:51 -0500, David Miller wrote: > From: Rainer Weikusat <[email protected]> > Date: Mon, 08 Feb 2016 18:47:19 +0000 > > > The present unix_stream_read_generic contains various code sequences of > > the form > > > > err = -EDISASTER; > > if () > > goto out; > > > > This has the unfortunate side effect of possibly causing the error code > > to bleed through to the final > > > > out: > > return copied ? : err; > > > > and then to be wrongly returned if no data was copied because the caller > > didn't supply a data buffer, as demonstrated by the program available at > > > > http://pad.lv/1540731 > > > > Change it such that err is only set if an error condition was detected. > > > > Fixes: 3822b5c2fc62 ("af_unix: Revert 'lock_interruptible' in stream > > receive code") > > Reported-by: Joseph Salisbury <[email protected]> > > Signed-off-by: Rainer Weikusat <[email protected]> > > Applied, thanks Rainer. > > And BTW I disagree with some of the feedback I saw in these threads > about "if (x) goto out;" being unreadable and that it should be avoided.
That's not what I said.
> That's completely wrong.
>
> Fact is, we've all been reading code of that form for multiple decades.
> So it's the style we are _MOST_ familiar with, and it is therefore the
> style that is the easiest and clearest for kernel developers to understand.
[...]
I agree that 'if (err) goto cleanup;' is widely used and is generally
understandable (though more creative uses of goto are often not).
My objection was to 'err = -EFOO; if (cond) goto cleanup;'. That is definitely
not clear and it hides mistakes like this.
Ben.
--
Ben Hutchings
Lowery's Law:
If it jams, force it. If it breaks, it needed replacing anyway.
signature.asc
Description: This is a digitally signed message part
