git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 10/11] builtin/gc: avoid global state in `gc_before_repack()`

From
Patrick Steinhardt <ps@pks.im>
Date
May 30, 2025, 14:05 UTC
Message-ID
<aDm7KdKrjhjDToqH@pks.im>
In-Reply-To
<6EE3E4F8-16DB-45AB-AD67-FADE12FBFBBD@gmail.com>
On Fri, May 30, 2025 at 08:56:36AM -0400, Ben Knoble wrote:
Show 18 quoted lines
> > @@ -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
Previous: Ben Knoble
Message 2 of 2 in “Re: [PATCH 10/11] builtin/gc: avoid global state in `gc_before_repack()`”
  1. Ben KnobleMay 30, 2025
  2. Patrick SteinhardtMay 30, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.