On Fri, May 30, 2025 at 08:56:36AM -0400, Ben Knoble wrote: > > @@ -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
