Re: [PATCH v2 3/3] git-compat-util: add NOT_A_CONST macro and use it in atfork_prepare()

> ---- >8 ----
> Our hope is that the number of code paths that falsely trigger
> warnings with the -Wunreachable-code compilation option are small,
> and they can be worked around case-by-case basis, like we just did
> in the previous commit.  If we need such a workaround a bit more
> often, however, we may benefit from a more generic and descriptive
> facility that helps document the cases we need such workarounds.
> 
>     Side note: if we need the workaround all over the place, it
>     simply means -Wunreachable-code is not a good tool for us to
>     save engineering effort to catch mistakes.  We are still
>     exploring if it helps us, so let's assume that it is not the
>     case.

> Introduce NOT_A_CONST() macro, with which, the developer can tell
> the compiler:
> 
>     Do not optimize this expression out, because, despite whatever
>     you are told by the system headers, this expression should *not*
>     be treated as a constant.

> --- a/Makefile
> +++ b/Makefile
> @@ -1018,6 +1018,7 @@ LIB_OBJS += ewah/ewah_bitmap.o
>  LIB_OBJS += ewah/ewah_io.o
>  LIB_OBJS += ewah/ewah_rlw.o
>  LIB_OBJS += exec-cmd.o
> +LIB_OBJS += fbtcdnki.o

> +/*
> + * Prevent an overly clever compiler from optimizing an expression
> + * out, triggering a false positive when building with the
> + * -Wunreachable-code option. false_but_the_compiler_does_not_know_it_
> + * is defined in a compilation unit separate from where the macro is
> + * used, initialized to 0, and never modified.
> + */
> +#define NOT_A_CONST(expr) ((expr) || false_but_the_compiler_does_not_know_it_)
> +extern int false_but_the_compiler_does_not_know_it_;

> diff --git a/run-command.c b/run-command.c
> index d527c46175..535c73a059 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -516,14 +516,12 @@ static void atfork_prepare(struct atfork_state *as)
>  	sigset_t all;
>  
>  	/*
> -	 * Do not use the return value of sigfillset(). It is transparently 0
> -	 * on some platforms, meaning a clever compiler may complain that
> -	 * the conditional body is dead code. Instead, check for error via
> -	 * errno, which outsmarts the compiler.
> +	 * POSIX says sitfillset() can fail, but an overly clever
> +	 * compiler can see through the header files and decide
> +	 * it cannot fail on a particular platform it is compiling for,
> +	 * triggering -Wunreachable-code false positive.
>  	 */
> -	errno = 0;
> -	sigfillset(&all);
> -	if (errno)
> +	if (NOT_A_CONST(sigfillset(&all)))
>  		die_errno("sigfillset");