Thread (2 messages) 2 messages, 2 authors, 2025-03-10

Re: [PATCH] config.mak.dev: enable -Wunreachable-code

From: Jeff King <hidden>
Date: 2025-03-10 16:04:49

On Mon, Mar 10, 2025 at 08:40:46AM -0700, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
On Fri, Mar 07, 2025 at 05:54:45PM -0500, Jeff King wrote:
quoted
However, clang does implement this option, and it finds the case
mentioned above (and no other cases within the code base). And since we
run clang in several of our CI jobs, that's enough to get an early
warning of breakage.
Hmph, this might be more trouble than it is worth.

After correcting the problem in the refs code, the osx CI builds (and
only those) now fail with:

  run-command.c:519:3: error: code will never be executed [-Werror,-Wunreachable-code]
                  die_errno("sigfillset");
                  ^~~~~~~~~
...
I guess a knob-less version is:

  errno = 0;
  sigfillset(&all); /* don't check return value! only errno */
  if (errno)
	die_errno("sigfillset");

which is subtle, to say the least.
Bah.  This is just as horrible as some other warnings that are not
enabled by default.  I guess we should just be more vigilant X-<.
Yeah. We could perhaps live with hacking around this one specific spot.
But there's an open question of how often these kinds of false positives
will come up.

Maybe not often, if there is only one instance in the current code base.
Or maybe a lot, but we wouldn't know because we haven't had the warning
enabled.

I guess another option is to enable it in _one_ CI job that uses clang
on Linux (maybe linux-sha256?) and see how often it is helpful or
harmful.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help