2013/12/9 Graham Bloice <[email protected]>

> On 8 December 2013 22:32, Pascal Quantin <[email protected]> wrote:
>
>> Hi Graham,
>>
>> Le 8 déc. 2013 à 22:56, Graham Bloice <[email protected]> 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 <[email protected]>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:[email protected]?subject=unsubscribe

Reply via email to