git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	git@vger.kernel.org, "Denton Liu" <liu.denton@gmail.com>,
	"Jeff Hostetler" <jeffhost@microsoft.com>
Subject: Re: [PATCH] trace2: refactor to avoid gcc warning under -O3
Date: Tue, 11 May 2021 17:07:24 -0400	[thread overview]
Message-ID: <YJryDGSliWvvABDB@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqlf8lkmwo.fsf@gitster.g>

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

  reply	other threads:[~2021-05-11 21:07 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-01  7:30 [PATCH] Fix -Wmaybe-uninitialized warnings under -O0 Denton Liu
     [not found] ` <CAPUEspgBkmxszgBee8C9hZnEwqztf-XKEj7LB_jWVFJaJCge0w@mail.gmail.com>
2020-04-01  9:05   ` Denton Liu
2020-04-01  9:52 ` Jeff King
2020-04-01 14:06   ` Denton Liu
2020-04-03 14:04     ` Jeff King
2020-04-03 14:38       ` Jeff King
2020-04-04 12:07       ` Denton Liu
2020-04-04 14:21         ` Jeff King
2021-05-05  8:40           ` [PATCH] trace2: refactor to avoid gcc warning under -O3 Ævar Arnfjörð Bjarmason
2021-05-05  9:47             ` Junio C Hamano
2021-05-05 13:34               ` Jeff King
2021-05-05 14:38             ` Johannes Schindelin
2021-05-06  1:26               ` Junio C Hamano
2021-05-06 20:29                 ` Johannes Schindelin
2021-05-06 21:10                   ` Junio C Hamano
2021-05-11 14:34                     ` Johannes Schindelin
2021-05-11 18:00                       ` Jeff King
2021-05-11 20:58                         ` Junio C Hamano
2021-05-11 21:07                           ` Jeff King [this message]
2021-05-11 21:33                             ` Junio C Hamano
2021-05-11  7:03             ` Junio C Hamano
2021-05-11 13:04             ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2021-05-11 16:40               ` Jeff Hostetler
2021-05-11 17:54               ` Jeff King
2021-05-11 18:08                 ` Jeff King
2021-05-11 21:09                   ` Junio C Hamano
2021-05-20  0:20                   ` Junio C Hamano
2021-05-20 11:05                     ` [PATCH v3] " Ævar Arnfjörð Bjarmason
2021-05-20 13:13                       ` Jeff King
2021-05-20 22:08                         ` Junio C Hamano
2021-05-21  9:34                           ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YJryDGSliWvvABDB@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=liu.denton@gmail.com \
    --subject='Re: [PATCH] trace2: refactor to avoid gcc warning under -O3' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).