On Fri, May 30, 2025 at 08:56:36AM -0400, Ben Knoble wrote:

quoted

@@ -965,7 +957,8 @@ int cmd_gc(int argc,
         goto out;
     }

-        gc_before_repack(&opts, &cfg); /* dies on failure */
+        if (gc_before_repack(&opts, &cfg) < 0)
+            exit(127);

If I (a relative novice to this part of the code) am reading
correctly, we trade an implicit die in a private helper for explicit
exit in a « main » function, which I find much easier to reason about.
Nice!

What I don’t see (being away from the rest of the source at the
moment) is where 127 comes from. I don’t intend a crusade against
magic numbers :) and I’ve certainly seen enough exit-codes of 127 to
guess what this means, but reading only the patch the number does
appear out of thin air.

The funny thing is that 127 isn't even correct -- it should be 128.
Maybe we can adapt `die_builtin()` so that it knows to not write
anything when the first argument is a NULL pointer?

Patrick