Re: [PATCH] trace2: refactor to avoid gcc warning under -O3

[Jeff King <peff@peff.net>]

On Wed, May 12, 2021 at 05:58:47AM +0900, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Tue, May 11, 2021 at 04:34:49PM +0200, Johannes Schindelin wrote:
> >
> >> On Fri, 7 May 2021, Junio C Hamano wrote:
> >> 
> >> > 		/* GCC thinks socket()/connect() might fail to set errno */
> >> > 		return errno ? errno : EIO;
> >> >
> >> > If a compiler thinks errno may *not* be set, can 'errno' be reliably
> >> > used to decide if it can be returned as-is or a fallback value EIO
> >> > should be used, without triggering the same (incorrect) working in
> >> > the first place?
> >> 
> >> Oh, I guess I mistook the problem for something else, then.
> >> 
> >> If the problem is that `errno` is not set in case of failure, the
> >> resolution is easy (and even recommended in the manual page of `errno`):
> >> simply set it to 0 before the syscall (or in the function that relies on
> >> `errno == 0` means success).
> >
> > I don't think that is the problem. According to the standard, errno is
> > always set to a non-zero value after a syscall failure.
> >
> > The problem is only that the compiler does not incorporate that special
> > knowledge of the variable, so it generates a warning even though we can
> > reason that the situation it describes is impossible.
> 
> Yes, that is what I tried to say (i.e. if the compiler does not know
> errno has a defined value at certain places in our code and
> complain, then "return errno ? errno : EIO" would get the same
> warning because bases its outcome on the value of errno the compiler
> thinks is possibly undefined at that point), but apparently I failed
> to convey that clearly enough.

One thing in what you said puzzles me, though. The problem is not that
the compiler thinks errno is not set at all (i.e., undefined). It simply
does not know whether it is non-zero or not after the call.

So switching to "return errno ? errno : EIO" does indeed work here,
because the compiler now knows that the result of that return will
always be non-zero.

It must assume that "errno" is always defined to _something_, because
it's a global (so there's no undefined behavior here). It was only it's
zero/non-zero implication that let to code-paths were "fd" could be
used uninitialized.

-Peff