git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Mar 27, 2023, 09:25 UTC
Message-ID
<8b236f96-4b1c-0784-3a25-becc20e3e806@gmx.de>
In-Reply-To
<pull.1438.v4.git.git.1674331462560.gitgitgadget@gmail.com>
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

Show 97 quoted lines
>
> 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
>
Previous: Rose via GitGitGadget
Message 5 of 5 in “mingw: replace deprecated GetVersion with RtlGetVersion”
  1. mingw: replace deprecated GetVersion with RtlGetVersionRose via GitGitGadget, Jan 19, 2023
  2. mingw: replace deprecated GetVersion with RtlGetVersionRose via GitGitGadget, Jan 19, 2023
  3. mingw: replace deprecated GetVersion with RtlGetVersionRose via GitGitGadget, Jan 21, 2023
  4. mingw: prefer RtlGetVersion over GetVersionRose via GitGitGadget, Jan 21, 2023
  5. Johannes SchindelinMar 27, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.