GNOME Bugzilla – Bug 789029
More compiler warnings in MINGW64 and MSYS2
Last modified: 2017-10-21 13:25:22 UTC
Created attachment 361640 [details] Fixes I described. I fixed some issues using a patch here that appear in the current GIT version of libxml2. But before discussing that, I need to point out that I also dis some things for MSYS2 which is somewhat of a CygWin-like environment. The differences are described at: https://github.com/msys2/msys2/wiki/How-does-MSYS2-differ-from-Cygwin . And the MSYS PKGBUILD's and patches are in this repository: https://github.com/Alexpux/MSYS2-packages . 1) In previous correspondences, I mentioned fall though and it seems to be deliberate. I modified configure.ac to determine if the appropriate is supported by GCC. Some old versions don't support the fallthrough warning so you can't use the appropriate flag with them. The change does use autoconf-archive. 2) #include wsockcompat.h has to come before an #include windows. This is because wsockcompat.h includes winsock2. 3) libxml/threads.h had to match exactly the IFDEF's and definitions in threads.c. The PKGBUILD for mingw-w64-libxml2 uses something like: CFLAGS="${CFLAGS} -DLIBXML_STATIC_FOR_DLL -DNOLIBTOOL" 4) libxml2/libxml.h - LIBXML_STATIC was being defined more than once creating warnings. 5) include/libxml/hash.h - the XML_CAST_FPTR macro was disabled due to a bug in GCC 4. I reenabled as per the comment and it seems to work here for GCC 6x and 7x. 6) libxml2/trionan.c - There were two unused procedures trio_is_special_quantity, and is_negative. Those were IFDEF'ed out. I saw something similar in libxml2/testapi.c as well as a direct reference to winsock2.h that you didn't want and has to come before windows.h. 7) libxml2/runtest.c - The thread was result was returning a pointer to a string in win32_thread_specific_data. The return value is a DWord that can NOT hold a pointer. I changed things in Windows so that thread_specific_data itself returns a DWord value that could then be compared against an Integer value when reporting results as opposed to comparing against string values. 8) libxml2/nanohttp.c - ai_addrlen should be compared against a value that is also a socklen_t such as "(socklen_t)sizeof(sockin))" 9) libxml2/testrecurse.c - This has some unused parameters in Windows so I IFDEF'ed out those parameters. I still get this even with the fixes I've done so far and I'm not sure what to do about this. ../libxml2/xmlmodule.c: In function 'xmlModulePlatformSymbol': ../libxml2/xmlmodule.c:349:13: warning: ISO C forbids assignment between function pointer and 'void *' [-Wpedantic] *symbol = GetProcAddress(handle, name); ^ BTW, I am convinced that you can actually support IPv6 in Windows so I will be looking further at this.
Created attachment 361641 [details] [review] MSYS2 patch to support the MSYS2 build chain - fixes configure.ac
1) This is caused by a change in GCC 7 that adds -Wno-implicit-fallthrough to -Wall or -Wextra. I'd prefer a patch that adds the fallthrough comments that -Wno-implicit-fallthrough requires. 2) testapi.c is a generated file, see the comment on top. Also, I'd prefer to define WIN32_LEAN_AND_MEAN before including windows.h which stops windows.h from including winsock.h. Anyway, the whole issue is probably caused by including windows.h in threads.h, see 3) 3) I'd prefer not to include windows headers from public headers unless absolutely necessary. Maybe we should change the function declarations to use plain C types: void* instead of LPVOID and HINSTANCE, unsigned long instead of DWORD. Looks good otherwise. 4) Looks good. 5) Just because it "seems to work" for you doesn't mean it's safe. Unfortunately, there's no way to disable this warning, which is part of -Wpedantic, selectively. We probably have to live with this warning for now. It also affects the UNIX build, see bug 300235. The GetProcAddress warning is the same underlying issue. 6) Also affects UNIX. In the long term, I'd like to get rid of Trio in the XPath code. This could also be fixed by upgrading to a newer version of Trio. For now, I'd prefer to keep third-party code as is. 7) I'd prefer to pass a struct argument to the threads that also contains the return value. 8) I don't think socklen_t is defined on every platform. Otherwise, there wouldn't be a need for the configure checks. It's probably best to cast both operands to size_t. 9) I'd prefer ATTRIBUTE_UNUSED instead.
Created attachment 361898 [details] [review] Updated patch
I did make an updated path that removes the hashes.h fix and the trioman stuff as well as the testapi.c stuff. That could only be fixed elsewhere. However, there are some things I need to mention. libxml2/runtest. - I had to keep that fix because the ThreadProc callback only returns a DWORD. That is 32-bit value even in Win64. I know that's what you wanted to hear. I redid the fix for DLLMain and xmlDllMain. xmlDllMain now has this signature: xmlDllMain(void *hinstDLL, unsigned long fdwReason, void *lpvReserved); and Ichangedthreads.c to match.The dword and hinstance don't seem to be interchangeable with unsigned long and void* in MINGW-w64.The signatures have to match in the prototype of the functions. Ditto for IFDEF's. Along this line, I also had to make a prototype for DLLMain but that is right in the C file itself. This is unusual but fixes the problem. I did not want the DLLMain definition in a public header. Right now, I'm working on some type of support for IPv6 in Windows. You do have the SOCKADDR_STORAGE, and getaddreinfo. The configure.ac will need to be adjusted for these.
I split up your patch and committed with some minor changes: https://git.gnome.org/browse/libxml2/log/ Getting IPv6 to work under Windows would be great.