Wireshark · Wireshark-dev: Re: [Wireshark-dev] Windows version info

[Graham Bloice <graham.bloice@xxxxxxxxxxxxx>]

On 9 December 2013 15:12, Pascal Quantin <pascal.quantin@xxxxxxxxx> wrote:

2013/12/9 Graham Bloice <graham.bloice@xxxxxxxxxxxxx>

On 8 December 2013 22:32, Pascal Quantin <pascal.quantin@xxxxxxxxx> wrote:

Hi Graham,

Le 8 déc. 2013 à 22:56, Graham Bloice <graham.bloice@xxxxxxxxxxxxx> a écrit :

Compiling with VS2013, the GetVersionEx function is now reported as deprecated:

E:\Wireshark\trunk\version_info.c(368): warning C4996: 'GetVersionExW': was declared deprecated [E:\Wireshark\2013build\wireshark.vcxproj]
E:\Wireshark\trunk\version_info.c(853): warning C4996: 'GetVersionExW': was declared deprecated [E:\Wireshark\2013build\wireshark.vcxproj]
E:\Wireshark\trunk\ui\win32\file_dlg_win32.c(451): warning C4996: 'Get VersionExW': was declared deprecated [E:\Wireshark\2013build\wireshark.vcxproj]

Normally with nmake builds we should have a flag removing this warning (see bug 9375 and revision 53059). Are you using cmake?

This was with CMake, however hiding the warning isn't really fixing the issue. 

I agree. On my side I see the new Microsoft API as being the major issue here :) By the way is cmake using the same manifest file than nmake? Otherwise Windows 8.1 detection will fail... :( (see bug 9298).

I'm working (slowly) on getting the manifests done in cmake.

 

 

Should we switch over to using the Version Helpers API and\or VerifyVersionInfo (and remove a lot of cruft from version_info.c)?  This has already been partially implemented in ui\win32\file_dlg.c as part of https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=9297

Version Helpers API: http://msdn.microsoft.com/en-us/library/windows/desktop/dn424972(v=vs.85).aspx

This is not what we need for version_info.c: this new API allows you to check a minimum OS requirement, not to get current OS version. See my comments in bugs 9297, 9298 and 9375 for more details.

In version_info,  get_os_version_info() just sets the stirng as required, using the Version Helpers we could step back from 8.1 down to Vista (or XP if we must) and stop when a version is found.  The IsWindowsServer() call allows us to differentiate between the two variants.

In get_os_major_version() the two callers I can see are checking for Vista or later, so that could be replaced with IsWindowsVistaOrGreater(), or more generically with IsWindowsVersionOrGreater(), changing the function to is_os_version_or_greater(versionRequired) or similar.

The call in win32_save_as_statstree() in file_dlg_win32.c could again be replaced with a call to is_os_version_or_greater(), or could be removed entirely as it's only for NT 4.0/Win95 compatibility of the OPENFILENAME structure ofn.

As I said, lots of cruft can be removed.

Well it's only removed for MSVC2013 and later, we need to keep existing code for earlier MSVC versions, which means twice the amount of work to update Wireshark for a new Windows version.

On my side I wanted to avoid the multiple calls to VerifyVersionInfo (version helpers are just a wrapper for it) that seemed overkill, but that's really a matter of taste and I'm not fully satisfied with the current code either (the manifest trick is kind of ugly...)

Using GetFileVersionInfo API might be the best tradeoff but I was too lazy to explore this path :)

I read MSDN again and now see that although the functions work on all the downlevel OS's, the include file that defines them is in the Win 8.1 SDK.  MSDN says that you can use the file with other versions of Visual Studio, but I don't think they'd take kindly to us distributing it.

This brings me on to my oft voiced compliant, in that our code shouldn't be checking the compiler version to make (some, most, all ??) these decisions, rather it should be checking the SDK in use, as earlier versions of the compilers can easily use much later SDK's