From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org, Taylor Blau <ttaylorr@openai.com>,
Derrick Stolee <stolee@gmail.com>
Subject: Re: [PATCH v2 1/7] banned-die: create header for banning of functions
Date: Mon, 31 Aug 2026 15:30:36 +0200 [thread overview]
Message-ID: <apWB_D9oivo56vcw@pks.im> (raw)
In-Reply-To: <xmqqh5kikkgi.fsf@gitster.g>
On Tue, Aug 25, 2026 at 01:34:53PM -0700, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Derrick Stolee <stolee@gmail.com>
> >
> > 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.
> >
> > It would be reasonable to name this file trace2/tr2_banned.h to be
> > specific to the trace2 API, but it seems like such a restriction would
> > be valuable to put in some other areas of the code, so adding it at the
> > root of the tree seems like a good long-term approach.
>
> In other words, the functions banned by including this file are not
> listed because they are banned from being used in trace2 API, but
> because they may lead to die(). There may be some other traits that
> we might want to avoid in certain subset of our code, and we may
> have similar banned-frotz.h header to prevent direct or indirect use
> of frotz. Which makes sense to me.
>
> Would the same approach work for the_hash_algo and the_repository, I
> wonder?
Don't we already do this? If `USE_THE_REPOSITORY_VARIABLE` is not
defined then we hide several function declarations where we know that
they depend on `the_repository`. It's not perfect as we still expose
functions that do rely on it implicitly, but it's easy to remove more
function declarations over time by just adding another ifdef.
Maybe we should follow a similar approach with functions that die?
Patrick
next prev parent reply other threads:[~2026-08-31 13:30 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-15 16:12 [PATCH] trace2: tolerate failed timestamp formatting Derrick Stolee via GitGitGadget
2026-07-17 16:24 ` Taylor Blau
2026-07-18 15:01 ` Derrick Stolee
2026-07-20 14:29 ` Junio C Hamano
2026-07-20 14:37 ` Taylor Blau
2026-07-29 21:35 ` Junio C Hamano
2026-07-31 13:26 ` Derrick Stolee
2026-07-31 15:57 ` Junio C Hamano
2026-08-25 18:56 ` [PATCH v2 0/7] trace2: stop allowing die() Derrick Stolee via GitGitGadget
2026-08-25 18:56 ` [PATCH v2 1/7] banned-die: create header for banning of functions Derrick Stolee via GitGitGadget
2026-08-25 20:34 ` Junio C Hamano
2026-08-31 12:28 ` Derrick Stolee
2026-08-31 13:30 ` Patrick Steinhardt [this message]
2026-08-25 22:14 ` Elijah Newren
2026-08-31 12:29 ` Derrick Stolee
2026-08-27 5:10 ` Jeff King
2026-08-31 12:38 ` Derrick Stolee
2026-08-25 18:56 ` [PATCH v2 2/7] trace2: tolerate failed timestamp formatting Derrick Stolee via GitGitGadget
2026-08-25 18:56 ` [PATCH v2 3/7] trace2: remove use of xstrdup() Derrick Stolee via GitGitGadget
2026-08-25 22:14 ` Elijah Newren
2026-08-31 12:41 ` Derrick Stolee
2026-08-25 18:56 ` [PATCH v2 4/7] trace2: remove use of ALLOC_ARRAY() Derrick Stolee via GitGitGadget
2026-08-25 18:56 ` [PATCH v2 5/7] trace2: remove use of xstrfmt() Derrick Stolee via GitGitGadget
2026-08-25 22:14 ` Elijah Newren
2026-08-25 22:36 ` Junio C Hamano
2026-08-31 12:51 ` Derrick Stolee
2026-08-25 18:56 ` [PATCH v2 6/7] trace2: remove use of ALLOC_GROW() Derrick Stolee via GitGitGadget
2026-08-25 22:14 ` Elijah Newren
2026-08-25 18:56 ` [PATCH v2 7/7] trace2: remove use of xcalloc() Derrick Stolee via GitGitGadget
2026-08-27 5:23 ` [PATCH v2 0/7] trace2: stop allowing die() Jeff King
2026-08-31 13:27 ` Derrick Stolee
2026-09-01 5:01 ` Jeff King
2026-09-01 5:03 ` Jeff King
2026-09-01 13:42 ` Derrick Stolee
2026-08-31 17:25 ` [PATCH v3 " Derrick Stolee via GitGitGadget
2026-08-31 17:25 ` [PATCH v3 1/7] banned-die: create header for banning of functions Derrick Stolee via GitGitGadget
2026-08-31 17:25 ` [PATCH v3 2/7] trace2: tolerate failed timestamp formatting Derrick Stolee via GitGitGadget
2026-08-31 17:25 ` [PATCH v3 3/7] trace2: remove use of xstrdup() Derrick Stolee via GitGitGadget
2026-08-31 17:25 ` [PATCH v3 4/7] trace2: remove use of ALLOC_ARRAY() Derrick Stolee via GitGitGadget
2026-08-31 17:25 ` [PATCH v3 5/7] trace2: remove use of xstrfmt() Derrick Stolee via GitGitGadget
2026-08-31 17:25 ` [PATCH v3 6/7] trace2: remove use of ALLOC_GROW() Derrick Stolee via GitGitGadget
2026-08-31 17:25 ` [PATCH v3 7/7] trace2: remove use of xcalloc() Derrick Stolee via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=apWB_D9oivo56vcw@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=stolee@gmail.com \
--cc=ttaylorr@openai.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox