Re: [PATCH v2 0/7] trace2: stop allowing die()

From: Jeff King <peff@peff.net>
Date: Sep 1, 2026, 05:01 UTC
Message-ID: <20260901050129.GB1075462@coredump.intra.peff.net>
In-Reply-To: <a41bdb3b-1fe7-4c1e-9d16-72390d93503b@gmail.com>

On Mon, Aug 31, 2026 at 09:27:49AM -0400, Derrick Stolee wrote:

Show 11 quoted lines

> > OK. This feels like the tip of the iceberg, though. All of strbuf would
> > have to be off-limits, too (both because it calls malloc directly, but
> > also because it will bail if snprintf() returns -1). I won't be
> > surprised if there are other indirect calls hiding in various places
> > (e.g., all of json-writer.c).
> 
> You're absolutely right. Not only in json-writer.c, but several direct
> calls to the strbuf API. The only real way to fix that would be to
> create a "safe strbuf" library. This is potentially an interesting
> direction that I might want to pursue and send an RFC after getting
> started.

Yes, though at some point the strbuf abstractions don't necessarily make sense, and you want to surface "did we truncate" or "did this result fit" to the caller.

So you probably end up with a whole new string interface (hopefully much more stripped down than what strbuf needs).

Show 5 quoted lines

> > I think if you really want to avoid allocations in trace2 it would
> > probably need to be a ground-up no-dependency rewrite.
> 
> Or to update the dependencies to be "safe". Not an easy thing, either
> way.

Yes. My thinking is that by the time you've pruned the dependencies, you've essentially done that rewrite. So maybe it is all just a matter of perspective. One man's refactor is another's rewrite, or something. :)

> I don't have much knowledge of CodeQL, but the following vibe-coded
> .ql script is able to detect these transitive calls and demonstrate
> the issue:

Yeah, I think the whack-a-mole can be solved with static analysis that actually understands the complete (possible) call tree. And then you wouldn't even really need your banned-die.h, because you'd have the real thing.

There's probably still a lot of work in rewriting the code to avoid those dependencies, though. And I fear you may hit some part that really needs to call into generic Git code in order to get an answer, which will be hard to pull apart. But maybe not; in theory we are feeding data into trace2, and it never really "asks" the rest of Git anything substantial.

-Peff