Thread (42 messages) flat view 42 messages, 7 authors, 5d ago

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

From: Jeff King <hidden>
Date: 2026-09-01 05:01:31

On Mon, Aug 31, 2026 at 09:27:49AM -0400, Derrick Stolee wrote:
quoted
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).
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help