[Spice-devel] [PATCH] Use enumeration for supported_system_version return type

[Frediano Ziglio]

> 
> Hi,
> 
> Patch looks good to me, small question bellow.
> Acked-by: Victor Toso <victortoso at redhat.com>
> 
> On Wed, Aug 10, 2016 at 08:17:57AM +0100, Frediano Ziglio wrote:
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  common/vdcommon.cpp | 6 +++---
> >  common/vdcommon.h   | 2 +-
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/common/vdcommon.cpp b/common/vdcommon.cpp
> > index 4f80a2c..b5b18ac 100644
> > --- a/common/vdcommon.cpp
> > +++ b/common/vdcommon.cpp
> > @@ -17,7 +17,7 @@
> >  
> >  #include "vdcommon.h"
> >  
> > -int supported_system_version()
> > +SystemVersion supported_system_version()
> >  {
> >      OSVERSIONINFOEX osvi;
> >  
> > @@ -25,14 +25,14 @@ int supported_system_version()
> >      osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX);
> >      if (!GetVersionEx((OSVERSIONINFO*)&osvi)) {
> >          vd_printf("GetVersionEx() failed: %lu", GetLastError());
> > -        return 0;
> > +        return SYS_VER_UNSUPPORTED;
> >      }
> >      if (osvi.dwMajorVersion == 5 && (osvi.dwMinorVersion == 1 ||
> >      osvi.dwMinorVersion == 2)) {
> >          return SYS_VER_WIN_XP_CLASS;
> >      } else if (osvi.dwMajorVersion == 6 && osvi.dwMinorVersion >= 0 &&
> >      osvi.dwMinorVersion <= 2) {
> >          return SYS_VER_WIN_7_CLASS;
> 
> Based on GetVersionEx docs [0], seems that this function changed/got
> deprecated. Might be a good opportunity to add a FIXME here?
> 
> [0]
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms724451(v=vs.85).aspx
> 
> Cheers,
>   toso
> 

I don't know, looks a bit OT.

Basically they reverted the logic of what an application should do.
Before Windows was supposed to be retro compatible and applications
should check Windows version if they want to use new features; now
the applications should report (using the manifest) to Windows the
versions they support so GetVersionEx became deprecated.

However I think for compatibility we still need to use this function
(when our application run in a system where Windows does not check
what version the application supports, for instance Windows XP/7).

A note on the patch: SYS_VER_UNSUPPORTED is (and was) 0 so this
should cause no byte changes, just syntax.

Frediano

> >      }
> > -    return 0;
> > +    return SYS_VER_UNSUPPORTED;
> >  }
> >  
> >  #ifndef HAVE_STRCAT_S
> > diff --git a/common/vdcommon.h b/common/vdcommon.h
> > index bc8ce33..970e6cc 100644
> > --- a/common/vdcommon.h
> > +++ b/common/vdcommon.h
> > @@ -99,7 +99,7 @@ enum SystemVersion {
> >      SYS_VER_WIN_7_CLASS,  // also Windows 8, Server 2012, Server 2008/R2 &
> >      Vista
> >  };
> >  
> > -int supported_system_version();
> > +SystemVersion supported_system_version();
> >  
> >  #endif
> >