GNOME Bugzilla – Bug 741895
gwin32: Add Simple API to check whether we are on at least a version of Windows
Last modified: 2015-01-27 04:26:55 UTC
Hi, As Windows 8.1 introduced some changes to GetVersion() and GetVersionInfoEx() [1][2], I think probably it would be a good idea if we have a new API in GLib to check whether we are at least running some specified version of Windows, including the service pack level and the type of Windows (i.e. workstation edition or server edition) where the code is being run on. This also enables Windows version checks to be done in a more careful manner. A few examples of which this might become useful: -We need a certain version of Windows for OS-level functionality (for those that we don't need to venture into the GetProcAddress() thing). -Some OS functionality changed in the way it worked, such as DWM (i.e. compositing) being always activated on Windows 8+/Server 2012+, and cannot be switched off. This is part of bug 741849, and hence 727316. [1]: http://msdn.microsoft.com/zh-tw/library/windows/desktop/ms724451%28v=vs.85%29.aspx [2]: http://msdn.microsoft.com/zh-tw/library/windows/desktop/ms724451%28v=vs.85%29.aspx With blessings, thank you!
Created attachment 293224 [details] [review] gwin32: Add public API g_win32_windows_is_at_least() Hi, This is my proposal for the code, with the additional note for the g_win32_get_windows_version() API due to the changes in GetVersion(). There are two things that I can think of regarding g_win32_get_windows_version() after this API addition: -Simply deprecate this in favor of the new API. -For Windows before 8.1 (checking via the new API), do things as before; otherwise retrieve the Windows version by calling the new API in a loop until it returns TRUE by going down a know versions list (or FALSE if we go the other way round). The catch to this is that we need to maintain the list upon releases of Windows and possibly service packs. With blessings, thank you!
Hi, Any comments or suggestions about this new, simple-ish API? Much appreciated. With blessings, thank you!
It looks like a nice API, although for consistency with other similar version check functions, it would be better named like "g_win32_check_windows_version". Also /G_WIN32_OS_WS/G_WIN32_OS_WORKSTATION would be nicer. It would nicely fit in gtk+/gdk/win32/gdkwindow-win32.c instead of this rather unfriendly call: if (LOBYTE (g_win32_get_windows_version()) > 0x05 || LOWORD (g_win32_get_windows_version()) == 0x0105) { /* Windows XP (5.1) or above */ wcl.style |= 0x00020000; /* CS_DROPSHADOW */ I don't see much other use in GNOME modules, spice-gtk does use it to set some security flags on named pipe with >= Vista. Tbh, I would make g_win32_get_windows_version() deprecated, since Microsoft doesn't recommend it: "[GetVersion may be altered or unavailable for releases after Windows 8.1. Instead, use the Version Helper APIs]" Other than that, looks like a nice proposal.
Created attachment 295061 [details] [review] gwin32: Add public API g_win32_check_windows_version() Hello Marc-Andre, (In reply to comment #3) > It looks like a nice API, Thanks, nice to hear that this could be useful. > ...it would be better named like "g_win32_check_windows_version". > Also /G_WIN32_OS_WS/G_WIN32_OS_WORKSTATION would be nicer. Noted with thanks, updated in the patch > I don't see much other use in GNOME modules, spice-gtk does use it to set some > security flags on named pipe with >= Vista. The other place I intend to use it is on DWM support in GTK+, as DWM is made standard and always activated for Windows 8 and later, which is used for composited windows. Hopefully this can be handy in other places and applications as well, as it wraps some parts of VerifyVersionInfo(). > "[GetVersion may be altered or unavailable for releases > after Windows 8.1. Instead, use the Version Helper APIs]" Thanks, and noted in patch. I re-worded some parts of the docs and made that API deprecated. This is one of the major reasons behind this new API. With blessings, thank you!
Review of attachment 295061 [details] [review]: Looks good to me, otherwise. ::: glib/gwin32.h @@ +142,3 @@ + G_WIN32_OS_WORKSTATION, + G_WIN32_OS_SERVER, +} GWin32OSType; The comment should be turned into a doc comment: /** * GWin32OSType: * @G_WIN32_OS_ANY:... ... Also, don't forget to add the the new symbol (and the enum) to the sections.txt file
Created attachment 295407 [details] [review] gwin32: Add public API g_win32_check_windows_version() (take ii) Hello Matthias, Here's the updated patch with the enum with GTK-Doc comments. With blessings, thank you!
Created attachment 295408 [details] [review] Update glib-sections.txt for g_win32_check_windows_version() Hello Matthias, As the name suggests, this is the update to glib-sections.txt. With blessings.
Review of attachment 295407 [details] [review]: looks good to me now.
Review of attachment 295408 [details] [review]: sure
Hello Matthias, Thanks for the reviews, the patches were committed as: -Attachment 295407 [details]: be2d9b4 -Attachment 295408 [details]: e07cc89 Will close this bug now. With blessings, thank you!