Re: [PATCH 2/5] uname: use only one OSVERSIONINFOEXA struct, use ANSI version

[Bruno Haible]

Hi Paolo,

> @@ -42,15 +43,20 @@ uname (struct utsname *buf)
>    if (gethostname (buf->nodename, sizeof (buf->nodename)) < 0)
>      strcpy (buf->nodename, "localhost");
>  
> -  /* Determine major-major Windows version.  */
> -  version.dwOSVersionInfoSize = sizeof (OSVERSIONINFO);
> -  have_version = GetVersionEx (&version);
> +  /* Determine major-major Windows version.  OSVERSIONINFOEXA starts 
> +     with the same fields as OSVERSIONINFOA, request those first.  */
> +  version.dwOSVersionInfoSize = sizeof (OSVERSIONINFOA);
> +  have_version = GetVersionExA ((OSVERSIONINFOA *) &version);

This chunk introduces some a-priori knowledge about the layout of
some OS structures, and introduces a cast. What for? For minimizing
the number of local variables of a function? I object.

>    if (have_version)
>      {
>        if (version.dwPlatformId == VER_PLATFORM_WIN32_NT)
>       {
>         /* Windows NT or newer.  */
>         super_version = "NT";
> +
> +       version.dwOSVersionInfoSize = sizeof (OSVERSIONINFOEXA);
> +       have_version = GetVersionExA ((OSVERSIONINFOA *) &version);
> +       assert (have_version);
>       }
>        else if (version.dwPlatformId == VER_PLATFORM_WIN32_WINDOWS)
>       {
> @@ -129,11 +135,7 @@ uname (struct utsname *buf)
>             }
>         else if (version.dwMajorVersion == 6)
>           {
> -           OSVERSIONINFOEX versionex;
> -
> -           versionex.dwOSVersionInfoSize = sizeof (OSVERSIONINFOEX);
> -           if (GetVersionEx ((OSVERSIONINFO *) &versionex)
> -               && versionex.wProductType != VER_NT_WORKSTATION)
> +           if (version.wProductType != VER_NT_WORKSTATION)
>               strcpy (buf->release, "Windows Server 2008");
>             else
>               switch (version.dwMinorVersion)

With this patch you initialize in line 60, inside some 'if's, a
variable which is used in line 130, inside some other nested 'if's.
This does not increase maintainability. Sorry.

If the "if (have_version)" should go away, can you please present
the patch that removes those "if (have_version)" _first_? Maybe the
other changes are then less objectionable.

Bruno