Re: [PATCH] compat/winansi: fix die_lasterr() argument formatting

[Junio C Hamano <gitster@pobox.com>]

Yongqiang Tian <yqtian668@gmail.com> writes:

> 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.

> 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