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

[Victor Toso]

Hi,

On Wed, Aug 10, 2016 at 05:32:43AM -0400, Frediano Ziglio wrote:
> > 
> > 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.

Ok, let it be

>
> 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).

Yes, that's why I suggested just a FIXME

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

Yep!
  toso

>
> > >      }
> > > -    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
> > >