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

[Paolo Bonzini]

On 10/02/2009 02:08 AM, Bruno Haible wrote:
> 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.

The knowledge is well-known (sorry for the pun).  It's not written 
clearly in the documentation (where you can only look up the definitions 
of the structs there), but it can be understood from the fact that you 
can pass an OSVERSIONINFO or OSVERSIONINFOEX indifferently to 
GetVersionEx.  So, the cast is kind-of a cast to base class.

The cast is present anyway below in the code that I moved here, so 
nothing new.

Besides, this patch is just preparatory and the actual code used is the 
one in 3/5 (first I refactor, then I modify the code).  The idea after 
that patch is to get the OSVERSIONINFOEX info is possible, else fall 
back to OSVERSIONINFO and zero out those fields.

I could have just presented "a rewrite" and could have tested it with 
all possible compatibility settings on Wine or native Windows; I split 
it to ease correctness review, but please look at maintainability of the 
final code only.

Paolo