NOTE: this v3 is rebased onto a recent 'master' due to conflicts in a test
script.

After v1 was posted, based on a concrete example of tracing leading to a
recursive die() problem, more evidence has come up to imply that allocations
are failing for some users more often. This is potentially an issue with the
allocator chosen by Git for Windows, which is being discussed elsewhere.

But the conclusion is this: the trace2 API shouldn't call helpers that might
call die(). It's too low-level for that.

In this v2, I have a much more robust approach to removing die() from the
trace2 API.

This starts with a new banned-die.h header file at the root of the repo and
including it from all trace2 API *.c files. It starts empty, but the later
patches will add one method at a time:

 * xsnprintf() : This is the original patch, but made more complete by
   adding the method to banned-die.h.
 * xstrdup()
 * ALLOC_ARRAY()
 * xstrfmt()
 * ALLOC_GROW()
 * xcalloc()

During each patch, the goal was to have the trace2 logic be "as correct as
possible" when an allocation failure occurs. This may mean that we have
incomplete messages or dropped trace messages.

The focus here is that the trace2 API should never cause a process-ending
failure, because those failures will trigger trace2 API calls while
reporting the failure.

Updates in V3
=============

 * Peff correctly points out that this is far from complete, as the strbuf
   library is not safe from die(). The banned-die.h provides incremental
   demonstration that these changes are showing progress and preventing
   regression in future changes, but not showing a complete picture. I will
   start an investigation into a "safe" or "gentle" variant of the strbuf
   API as a potential direction for these API layers.
 * The first patch had a lowercase banned() that should have been uppercase
   BANNED().
 * A 'return -1' was replaced with 'return 0' to avoid a misleading error
   message.
 * The ":<REDACTED>" string length was incorrect. This is fixed and tests
   are improved to cover this string manipulation. These test changes
   conflict with changes to use test_grep in 47f79f61983 (t: convert grep
   assertions to test_grep, 2026-07-06), so this v3 is rebased onto
   'master'.
 * Patch 6 was previously failing at runtime. The appropriate fix is pulled
   out of patch 7 and into patch 6.

Thanks, -Stolee

Derrick Stolee (7):
  banned-die: create header for banning of functions
  trace2: tolerate failed timestamp formatting
  trace2: remove use of xstrdup()
  trace2: remove use of ALLOC_ARRAY()
  trace2: remove use of xstrfmt()
  trace2: remove use of ALLOC_GROW()
  trace2: remove use of xcalloc()

 banned-die.h            | 32 +++++++++++++++++
 t/t0212-trace2-event.sh | 12 ++++---
 trace2.c                | 52 +++++++++++++++++++++++++---
 trace2/tr2_cfg.c        |  2 ++
 trace2/tr2_cmd_name.c   |  2 ++
 trace2/tr2_ctr.c        | 12 ++++++-
 trace2/tr2_dst.c        |  2 ++
 trace2/tr2_sid.c        |  2 ++
 trace2/tr2_sysenv.c     |  8 +++--
 trace2/tr2_tbuf.c       | 51 +++++++++++++++++++--------
 trace2/tr2_tgt_event.c  |  2 ++
 trace2/tr2_tgt_normal.c |  2 ++
 trace2/tr2_tgt_perf.c   |  2 ++
 trace2/tr2_tls.c        | 77 +++++++++++++++++++++++++++++++++++++++--
 trace2/tr2_tls.h        |  7 ++++
 trace2/tr2_tmr.c        | 16 +++++++--
 16 files changed, 250 insertions(+), 31 deletions(-)
 create mode 100644 banned-die.h

base-commit: c73e85354c275c9d409b26445089bc16940fc527

Submitted-As: https://lore.kernel.org/git/pull.2178.v3.git.1788197143.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.2178.git.1784131932489.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.2178.v2.git.1787684181.gitgitgadget@gmail.com