Re-explaining that errno thing....

[Jeremy Allison]

On Mon, Feb 27, 2017 at 09:19:25AM +0100, Michael Adam wrote:
> I personally find the old pattern more obvious, in the sense
> that the errno bracket around the cleanup code is much more
> obvious.
> 
> But it has 2 issues:
> 
> 1) As Uri said, the erroring-out code must make sure that
>    between the failed call and the goto out, there is no
>    errno-changing call.
> 2) The way it is written, it may be setting errno in cases
>    where it would not be needed (errno == 0).
>    This could of course be fixed with adding if (saved_errno) ...
>    as in the old pattern.
>    On the other hand side, this may be desired if for instance
>    the failing operation did not produce an errno, but the
>    cleanup code did set an errno... Not sure what's better
>    in this situation.
> 
> The new pattern has issues:
> 
> 1) Because errno handling is scattered, it duplicates code
>    and is less obvious (to me). And the programmer must
>    always remember to save the errno at each failure point.

Nope. Remember, this pattern is to save the errno at
the *relevent* failure point on a list of multiple
syscalls, only *one* of which should have errors returned
to the caller. If all potential errno's should be returned
you don't need to do anything at all except 'goto out'
on error.

> 2) The reverse of issue #2 above: If there was no errno
>    in the originally failing code, but there is an errno
>    in the cleanup code, then the cleanup code's errno will
>    be set at exit. That may or may not be desired.
> 
> So, I'd say neither approach is perfect. I personally
> tend to prefer a variant of the old pattern.

Not worth a nickel :-). I really dislike the old
pattern, as it is very unclear as to what and
when preserves/restores errno.

You always set errno from saved errno as the
last thing before returning.

If the call returns a (posix-style) -1 error,
then errno is valid. If it returns success
then you don't look and shouldn't assume
anything about errno.

Anyway, this has descended into bike-shedding
now (IMHO). We have much more urgent stuff to
fix :-).