Re: [PATCH v2 1/7] banned-die: create header for banning of functions

From: Jeff King <peff@peff.net>
Date: Aug 27, 2026, 05:10 UTC
Message-ID: <20260827051053.GB176544@coredump.intra.peff.net>
In-Reply-To: <84634717e2eca479026d1cdf39a089a8f61d131e.1787684181.git.gitgitgadget@gmail.com>

On Tue, Aug 25, 2026 at 06:56:15PM +0000, Derrick Stolee via GitGitGadget wrote:

Show 10 quoted lines

> We have universally-banned functions listed in banned.h since
> c8af66ab8ad (automatically ban strcpy(), 2018-07-26), but some layers of
> the code should be more strict than others.
> 
> One such example is the trace2 API which runs during atexit() and can
> prove to cause die()-handler recursion problems if it calls die().
> 
> Create a new banned-die.h header file that will ban some Git methods
> that call die(). Include that in all trace2 API implementation files.
> This currently only bans die() itself, and that was already not used.

There's a subtle but big difference between the universal code bans in banned.h and this banned-die.h. In the former case we are deciding strcpy() is unfit for our code base and outlawing it everywhere. The potential problem is in the source code, so catching it while compiling the source code is OK.

But we are not doing that with die(). It is a perfectly OK function in general, but we do not want to ever trigger its runtime effects from certain code paths. Banning it from being called from those code paths can catch _some_ instances, but not any transitive calls. If we call foo(), it may call die() itself, and we would not want to ban foo() from doing so. And recursively for functions called by foo() and so on.

So you end up playing whack-a-mole with functions that might call die() and adding them to this ban list.

I think that's _probably_ the best we can do in practice. I think the framing above suggests that we could approach the problem more directly with a runtime flag: when we enter those code paths, set a flag to avoid the unwanted behavior, and have the low-level code respect that. But die() is a special case here, because we'd want to suppress its no-return behavior. And its callers are not prepared for die() to suddenly start returning because of some global flag.

So I think the whack-a-mole is the best we can do. But I would not want to see this strategy extended to other areas. In most cases some kind of runtime support is probably a better solution.

-Peff