Re-explaining that errno thing....
Jeremy Allison
jra at samba.org
Mon Feb 27 16:44:20 UTC 2017
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 :-).
More information about the samba-technical
mailing list