Home page logo
/

wireshark logo Wireshark mailing list archives

Re: Windows version info
From: Pascal Quantin <pascal.quantin () gmail com>
Date: Mon, 9 Dec 2013 16:12:57 +0100

2013/12/9 Graham Bloice <graham.bloice () trihedral com>

On 8 December 2013 22:32, Pascal Quantin <pascal.quantin () gmail com> wrote:

Hi Graham,

Le 8 déc. 2013 à 22:56, Graham Bloice <graham.bloice () trihedral com> 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).




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

Pascal
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]