Re: [PATCH] trace2: tolerate failed timestamp formatting
- From
Taylor Blau <ttaylorr@openai.com>
- Date
- Jul 17, 2026, 16:24 UTC
- Message-ID
- <alpXW5U6sndZtgqV@com-79390>
- In-Reply-To
- <pull.2178.git.1784131932489.gitgitgadget@gmail.com>
On Wed, Jul 15, 2026 at 04:12:11PM +0000, Derrick Stolee via GitGitGadget wrote:
> This change removes all uses of xsnprintf() from the trace2/ directory. > There are two uses of xstrdup() that could be considered for removal, > but they only die() on out-of-memory errors instead of formatting > issues. I chose to leave those in place for now.
I may be missing some Git for Windows context, but I dug into this a little and I'm not sure 'gettimeofday()' is the culprit...
In my understanding Git for Windows's 'gettext.h' appears[1] to redirect the 'vsnprintf()' inside 'xsnprintf()' to 'libintl_vsnprintf()'. In this case, we have seven '%' placeholders. Gettext can store only six plus its end marker inline, so parsing the seventh causes an allocation before any timestamp values are read.
A failure there would produce the observed -1, after which 'xsnprintf()' dies and trace2 can recurse.
I think that also explains why calling 'snprintf()' directly helps. tr2_tbuf.c doesn't include gettext.h, so I think it bypasses libintl. If I'm reading compat/mingw.c correctly, 'gettimeofday()' fills tv and always returns zero [2], making the zero-initialization unrelated.
Would it make more sense to fix the xsnprintf()/libintl boundary and treat Trace2 reentrancy separately? I still can't explain why the allocation failed, so there may be another GfW-specific piece I’m missing.
I think something like the following (untested) would prevent the redirection to `libintl_vsnprintf()`:
--- 8< ---
diff --git a/wrapper.c b/wrapper.c index 16f5a63fbb..2976d4e110 100644 --- a/wrapper.c +++ b/wrapper.c @@ -7,7 +7,14 @@ #include "git-compat-util.h" #include "abspath.h" #include "parse.h" + +/* + * xsnprintf() only formats non-translated strings. On MinGW, avoid + * redirecting its vsnprintf() call to libintl's allocating replacement. + */ +#define _INTL_NO_DEFINE_MACRO_VSNPRINTF #include "gettext.h" +#undef _INTL_NO_DEFINE_MACRO_VSNPRINTF #include "strbuf.h" #include "trace2.h" --- >8 --- Thanks, Taylor [1]: https://github.com/git-for-windows/git-sdk-64/blob/1351ad2fc39a1f74c56b2cc2b38107ec8df8eb40/mingw64/include/libintl.h#L731-L754 [2]: https://github.com/microsoft/git/blob/vfs-2.55.0/compat/mingw.c#L1609-L1618