Re: [PATCH] compat/winansi: fix die_lasterr() argument formatting
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 16, 2026, 04:29 UTC
- Message-ID
- <xmqqh5jpyg2p.fsf@gitster.g>
- In-Reply-To
- <20260916042312.35891-1-yqtian668@gmail.com>
Yongqiang Tian <yqtian668@gmail.com> writes:
Show 5 quoted lines
> During WinANSI initialization, duplicate_handle() reports the handle
> when DuplicateHandle() fails:
>
> die_lasterr("DuplicateHandle(%li) failed", ...);
> ...I do not know about Patrick, but I do not do Windows, so please do not Cc: me a patch that is primarily about Windows portability.
I'll add two whose with contributions much greater than I have in the area to Cc: list.
Thanks.
Show 58 quoted lines
> die_lasterr() collects the formatting arguments in a va_list, but
> passes that va_list to die_errno() as an ordinary variadic argument.
> die_errno() consequently formats the representation of the va_list
> instead of the supplied handle, producing an incorrect fatal message.
> The other current callers pass fixed strings and are unaffected.
>
> Git does not provide a va_list-taking variant of die_errno(), so format
> the caller's arguments separately with strbuf_vaddf(). This consumes the
> original va_list correctly and produces the complete diagnostic prefix,
> including the handle supplied by duplicate_handle().
>
> Save GetLastError() before formatting because calls made while growing
> the strbuf may change the thread's Windows error value. Convert the
> saved value to errno only after formatting, then pass the completed
> message to die_errno() through a literal "%s". This prevents any percent
> characters in the formatted message from being interpreted a second
> time, while allowing die_errno() to append the corresponding system
> error and terminate as before.
>
> The updated compat/winansi.c compiles with MinGW GCC 13. A Win64 probe
> under Wine prints a value derived from the va_list before this change
> and the supplied integer afterward.
>
> Signed-off-by: Yongqiang Tian <yqtian668@gmail.com>
> ---
> compat/winansi.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/compat/winansi.c b/compat/winansi.c
> index 3ce190093..5547192a2 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -7,6 +7,7 @@
> #define DISABLE_SIGN_COMPARE_WARNINGS
>
> #include "../git-compat-util.h"
> +#include "../strbuf.h"
> #include <wingdi.h>
> #include <winreg.h>
> #include "win32.h"
> @@ -438,11 +439,15 @@ static void winansi_exit(void)
>
> static void die_lasterr(const char *fmt, ...)
> {
> + DWORD err = GetLastError();
> + struct strbuf message = STRBUF_INIT;
> va_list params;
> +
> va_start(params, fmt);
> - errno = err_win_to_posix(GetLastError());
> - die_errno(fmt, params);
> + strbuf_vaddf(&message, fmt, params);
> va_end(params);
> + errno = err_win_to_posix(err);
> + die_errno("%s", message.buf);
> }
>
> #undef dup2