On Mon, Apr 12, 2021 at 01:02:17PM +0200, Ævar Arnfjörð Bjarmason wrote:
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 9ddf9d7044b..540aba22a4d 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -480,10 +480,15 @@ void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
>
> /*
> * Let callers be aware of the constant return value; this can help
> - * gcc with -Wuninitialized analysis. We restrict this trick to gcc, though,
> - * because some compilers may not support variadic macros. Since we're only
> - * trying to help gcc, anyway, it's OK; other compilers will fall back to
> - * using the function as usual.
> + * gcc with -Wuninitialized analysis.
> + *
> + * We restrict this trick to gcc, though, because while we rely on the
> + * presence of C99 variadic macros, this code also relies on the
> + * non-standard behavior of GCC's __VA_ARGS__, allowing error() to
> + * work even if no format specifiers are passed to error().
> + *
> + * Since we're only trying to help gcc, anyway, it's OK; other
> + * compilers will fall back to using the function as usual.
> */
> #if defined(__GNUC__)
I don't mind leaving this gcc-only, since as you note that's the point
of what the code is trying to do. But wouldn't this always work because
we know there is at least one arg (the format itself)?
I.e., if we had written:
#define error(fmt, ...) (error(fmt, __VA_ARGS__), const_error())
that would be a problem for:
error("foo");
But because we wrote:
#define error(...) (error(__VA_ARGS__), const_error())
then it's OK.
-Peff
