On Fri, Mar 14, 2025 at 10:40:24AM -0700, Junio C Hamano wrote:
> >> -- >8 --
> >> Subject: [PATCH] run-command: use errno to check for sigfillset() error
> >>
> >> Since enabling -Wunreachable-code, builds with clang on macOS now fail,
> >> complaining that the die_errno() call in:
> >>
> >> if (sigfillset(&all))
> >> die_errno("sigfillset");
> >>
> >> is unreachable. On that platform the manpage documents that sigfillset()
> >> always returns success, and presumably the implementation is a macro or
> >> inline function that does so in a way that is transparent to the
> >> compiler.
> >
> > Would it work to instead do this here
> > ...
>
> I forgot to say a more important thing. Between the "let's excempt
> developers on macOS" and the "let's see how far we can go with the
> warning turned on everywhere and wack-a-mole this particular one
> with errno check" patches, I prefer the latter at least for a short
> term.
That's my gut feeling, too. I wasn't sure how people would feel about
actually touching the code (whereas the other patches were purely
turning compiler knobs). It may turn into wack-a-mole, but finding out
is part of the experiment.
Your CAN_BE_TAKEN() approach is certainly less subtle, and can be
applied in a more general way. If this is the only spot needed it may be
overkill, but the readability improvement alone probably makes it
worthwhile.
Do you want to turn that into a patch?
-Peff
