Hi Andrew,
Setting _WIN32_WINNT should just be restricting the available windows API functions you are prepared to access from the windows SDK you are compiling against. It doesn’t (ususally) trigger a change in behaviour
of an existing function, but that is something I have double checked. Certainly the documentation for the windows functions called in these files do not mention any change in behaviour between version, other than a caveat on GetVersionEx() that it is being
deprecated because it may not return quite what you think for later versions of windows. The current check for “thisIsNT” will return true for windows 2000 and above, so given the intended support targets of base this check could now be dropped and the code
simplified too.
When re-checking the original files my only area of concern is with SetThreadName.cpp . The code here says it was taken from a visual c++ example, I located a newer version of this example at
https://docs.microsoft.com/en-us/visualstudio/debugger/how-to-set-a-thread-name-in-native-code?view=vs-2019 which seems to have added pack pragmas and use sizeof(ULONG_PTR) rather than sizeof(DWORD) which makes me think the existing implementation in base
may not be 64bit safe. The original base code also only defines _WIN32_WINNT to be 0x400 on 64bit windows, which seems odd as windows that far back wasn’t 64bit! It may be that this is a trick that makes it work on 64bit windows? Given this slight oddness
it is probably best to update the code in this file to that from the link, and also to look at using the mentioned SetThreadDescription() function on more recent windows versions. I am happy to look at this and submit a patch.
The only other change that would happen with removing the _WIN32_WINNT defines is that there is a call to GetModuleHandleEx() in osdThread.c that is protected by “if _WIN32_WINNT >= 0x0501” and so would previously
never been used with _WIN32_WINNT being set to 0x0400. If base will now only support XP / 2003 and later, the second part of this conditional could be removed too.
Given I’ve removed the _WIN32_WINNT defines from our code here already, maybe the best thing is for me to report back in a few weeks time as to whether I’ve had any problems. The code in SetThreadName.cpp is
only ever used on debug builds so would not affect any deployments here, I’ll test it separately and also conditionally enable a SetThreadDescription() option on newer windows builds
Regards,
Freddie
Hi Freddie,
On 5/15/19 5:28 AM, Freddie Akeroyd - UKRI STFC via Core-talk wrote:
I was recently compiling EPICS base 3.15 for an earlier windows version from Windows 10 and had a compile error with the WIN32 base implementation
of SetThreadName.cpp. The file always sets _WIN32_WINNT to 0x400 (Specifying Windows NT 4.0, the comment says it is dropping support for W95!) which then conflicted with my setting of NTDDI_VERSION. The _WIN32_WINNT macro is also set to 0x400 in osdMutex.c
and osdThread.c but in these files it is only set if not otherwise defined externally. A similar check for an existing definition in SetThreadName.cpp would avoid my compile issue, but as Windows 95 and NT 4.0 are long in the past I wonder if removing these
redefinitions of _WIN32_WINNT from base would now be appropriate
If you think we can safely remove all those tests and definitions from libCom/osi/os/WIN32 for the "recent-enough" versions of Windows we should
support I'd be fine with doing that.
I just tried it on windows-x64 and windows-x64-mingw builds and MSVC gave me a deprecation warning for the
GetVersionEx() call in
epicsMutexOsdCreate() — could we remove the
thisIsNT variable and
CreateMutex() stuff from osdMutex.c as well while we're cleaning this up? It looks
like the required CRITICAL_SECTION support is present back as far as Windows XP
and Windows Server 2003, which is fine as far as I'm concerned.
Thanks,
- Andrew
--
Complexity comes for free, Simplicity you have to work for.