lkml.org 
[lkml]   [2016]   [Feb]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] af_unix: Don't set err in unix_stream_read_generic unless there was an error
From
Date
On Tue, 2016-02-16 at 12:51 -0500, David Miller wrote:
> From: Rainer Weikusat <rweikusat@mobileactivedefense.com>
> 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 <joseph.salisbury@canonical.com>
> > Signed-off-by: Rainer Weikusat <rweikusat@mobileactivedefense.com>
>
> 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.[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2016-02-17 01:41    [W:1.419 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site