Re: [PATCH v4] mingw: prefer RtlGetVersion over GetVersion

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Sat, 21 Jan 2023, Rose via GitGitGadget wrote:

> From: Seija Kijin <doremylover123@gmail.com>
>
> The previous way is deprecated and returns

Let's always provide references for such claims (which would otherwise
risk suspicion for being taken out of thin air).
https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getversion
has this to say:

	*GetVersion* may be altered or unavailable for releases after
	Windows 8.1.

The suggestion is to switch to using version helpers, but those version
helpers do not get you a version number, they instead help you determine
which major version you're on, which is not what we need.

Please note that the documentation in no way or form suggests to switch to
using `RtlGetVersion()`.

> the wrong value in Windows 8 and up,
> returning the manifest Windows data
> as opposed to the actual Windows data.

This is correct only if you ignore the reality that
`compat/win32/git.manifest` contains the values required to make it work.

> RtlGetVersion is the correct way
> to get the Windows version now.

Here, we would need a reference to support that claim. Seeing as I failed
to find any, I suspect that the claim can safely be considered to be
refuted and therefore the patch must be taken into doubt.

Ciao,
Johannes

>
> Note: ntdll does not need to be
> manually loaded into the runtime,
> as this is the one special library
> that is automatically loaded upon launch.
>
> Signed-off-by: Seija Kijin <doremylover123@gmail.com>
> ---
>     mingw: prefer RtlGetVersion over GetVersion
>
>     GetVersion has its behavior changed in Windows 8 and above anyway, so
>     this is the right way to do it now.
>
>     The previous way returns the wrong value in Windows 8 and up, returning
>     the manifest Windows data as opposed to the actual Windows data.
>
>     Signed-off-by: Seija Kijin doremylover123@gmail.com
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1438%2FAtariDreams%2Fmingw-v4
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1438/AtariDreams/mingw-v4
> Pull-Request: https://github.com/git/git/pull/1438
>
> Range-diff vs v3:
>
>  1:  31f778a6b34 ! 1:  8293e868970 mingw: replace deprecated GetVersion with RtlGetVersion
>      @@ Metadata
>       Author: Seija Kijin <doremylover123@gmail.com>
>
>        ## Commit message ##
>      -    mingw: replace deprecated GetVersion with RtlGetVersion
>      +    mingw: prefer RtlGetVersion over GetVersion
>
>           The previous way is deprecated and returns
>           the wrong value in Windows 8 and up,
>
>
>  compat/mingw.c | 39 +++++++++++++++++++++++++++++++++------
>  1 file changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index af397e68a1d..b1d75c93cfe 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -3081,15 +3081,42 @@ int wmain(int argc, const wchar_t **wargv)
>  	return exit_status;
>  }
>
> +/*
> + * for RtlGetVersion in uname
> + */
> +
> +typedef NTSTATUS(WINAPI *RtlGetVersionPtr)(PRTL_OSVERSIONINFOW);
> +union winprocaddr {
> +	FARPROC procaddr;
> +	RtlGetVersionPtr procGetVersion;
> +};
> +
>  int uname(struct utsname *buf)
>  {
> -	unsigned v = (unsigned)GetVersion();
> +	union winprocaddr RtlGetVersionInternal;
> +	OSVERSIONINFOW version;
> +
> +	memset(&version, 0, sizeof(version));
> +	version.dwOSVersionInfoSize = sizeof(version);
> +
> +	/* RtlGetVersion always gets the true Windows version, even when running
> +	 * under Windows's compatibility mode*/
> +	RtlGetVersionInternal.procaddr =
> +		GetProcAddress(GetModuleHandleW(L"ntdll.dll"), "RtlGetVersion");
> +
> +	if (RtlGetVersionInternal.procaddr) {
> +		RtlGetVersionInternal.procGetVersion((PRTL_OSVERSIONINFOW)&version);
> +	} else {
> +		/* Should not happen, but just in case, fallback to deprecated
> +		 * GetVersionExW */
> +		GetVersionExW(&version);
> +	}
> +
>  	memset(buf, 0, sizeof(*buf));
>  	xsnprintf(buf->sysname, sizeof(buf->sysname), "Windows");
> -	xsnprintf(buf->release, sizeof(buf->release),
> -		 "%u.%u", v & 0xff, (v >> 8) & 0xff);
> -	/* assuming NT variants only.. */
> -	xsnprintf(buf->version, sizeof(buf->version),
> -		  "%u", (v >> 16) & 0x7fff);
> +	xsnprintf(buf->release, sizeof(buf->release), "%lu.%lu",
> +		  version.dwMajorVersion, version.dwMinorVersion);
> +	xsnprintf(buf->version, sizeof(buf->version), "%lu",
> +		  version.dwBuildNumber);
>  	return 0;
>  }
>
> base-commit: 904d404274fef6695c78a6b055edd184b72e2f9b
> --
> gitgitgadget
>