On Tue, Jun 03, 2025 at 08:24:59AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
>
> > Sometimes code wants to die in a situation where it already has written
> > an error message. To use the same error code as `die()` we have to open
> > code the code with a call to `exit(128)` in such cases, which is easy to
> > get wrong and leaves magical numbers all over our codebase.
> > ...
> > if (!patch_format) {
> > fprintf_ln(stderr, _("Patch format detection failed."));
> > - exit(128);
> > + die(NULL);
>
> It is somewhat surprising that the compiler would not complain with
> "Hey, a NULL string as printf format string???" given its decl.
>
> NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
>
> As long as we are sure that compilers we care about are OK with
> this, it is a very nice ergonomics enhancement.
Hard to say. The documentation of -Wformat doesn't explicitly mention
what is expected to happen in that case. I tried compiling with
-Wformat=2, and that didn't generate any warnings for those calls to
`die(NULL)`. It did generate a bunch of unrelated warnings though.
I haven't seen any issues in our CI, either.
So I'm inclined to just go with this variant for now. If we ever see
that it does cause problems with some compiler it's trivial to just
`s/die(NULL)/die_silent()` or something similar.
Patrick
