Fix the windows build by fixing printf related issues #641

cmorty · 2023-11-16T13:40:31Z

This merge-request removes the need for

    #pragma GCC diagnostic ignored "-Wformat"
    #pragma GCC diagnostic ignored "-Wformat-extra-args"

in mingw. It also turns on -Wformat-signedness for GNU-compatible compilers.

This fixes #632

mcuee · 2023-11-16T14:27:09Z

This PR seems to work fine with MinGW build, from github action and from my local build.

My local build.

MINGW64 /c/work/hid/hidapi_pr641
$ cat build_verbose.sh
cmake -B build_mingw -D CMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_BUILD_TYPE=RelWithDebInfo -DHIDAPI_WITH_TESTS=ON -DHIDAPI_BUILD_PP_DATA_DUMP=ON -DCMAKE_C_FLAGS="-Wall -Wextra -pedantic -Werror"
cmake --build build_mingw

$ . ./build_verbose.sh
-- Building for: Ninja
CMake Deprecation Warning at CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 3.5 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.


-- The C compiler identification is GNU 13.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/msys64/mingw64/bin/cc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- hidapi: v0.14.0
-- Configuring done (0.8s)
-- Generating done (0.0s)
-- Build files have been written to: C:/work/hid/hidapi_pr641/build_mingw
Change Dir: 'C:/work/hid/hidapi_pr641/build_mingw'

Run Build Command(s): C:/msys64/mingw64/bin/ninja.exe -v
[1/8] C:\msys64\mingw64\bin\windres.exe -O coff -Dhidapi_winapi_EXPORTS -I C:/work/hid/hidapi_pr641/hidapi -I C:/work/hid/hidapi_pr641/windows  C:/work/hid/hidapi_pr641/windows/hidapi.rc src/windows/CMakeFiles/hidapi_winapi.dir/hidapi.rc.obj
[2/8] C:\msys64\mingw64\bin\cc.exe  -IC:/work/hid/hidapi_pr641/hidapi -IC:/work/hid/hidapi_pr641/windows -Wall -Wextra -pedantic -Werror -O2 -g -DNDEBUG -std=gnu11 -MD -MT src/windows/test/CMakeFiles/hid_report_reconstructor_test.dir/hid_report_reconstructor_test.c.obj -MF src\windows\test\CMakeFiles\hid_report_reconstructor_test.dir\hid_report_reconstructor_test.c.obj.d -o src/windows/test/CMakeFiles/hid_report_reconstructor_test.dir/hid_report_reconstructor_test.c.obj -c C:/work/hid/hidapi_pr641/windows/test/hid_report_reconstructor_test.c
[3/8] C:\msys64\mingw64\bin\cc.exe -Dhidapi_winapi_EXPORTS -IC:/work/hid/hidapi_pr641/hidapi -IC:/work/hid/hidapi_pr641/windows -Wall -Wextra -pedantic -Werror -O2 -g -DNDEBUG -MD -MT src/windows/CMakeFiles/hidapi_winapi.dir/hid.c.obj -MF src\windows\CMakeFiles\hidapi_winapi.dir\hid.c.obj.d -o src/windows/CMakeFiles/hidapi_winapi.dir/hid.c.obj -c C:/work/hid/hidapi_pr641/windows/hid.c
[4/8] C:\msys64\mingw64\bin\cc.exe -Dhidapi_winapi_EXPORTS -IC:/work/hid/hidapi_pr641/hidapi -IC:/work/hid/hidapi_pr641/windows -Wall -Wextra -pedantic -Werror -O2 -g -DNDEBUG -MD -MT src/windows/CMakeFiles/hidapi_winapi.dir/hidapi_descriptor_reconstruct.c.obj -MF src\windows\CMakeFiles\hidapi_winapi.dir\hidapi_descriptor_reconstruct.c.obj.d -o src/windows/CMakeFiles/hidapi_winapi.dir/hidapi_descriptor_reconstruct.c.obj -c C:/work/hid/hidapi_pr641/windows/hidapi_descriptor_reconstruct.c
[5/8] cmd.exe /C "cd . && C:\msys64\mingw64\bin\cc.exe -Wall -Wextra -pedantic -Werror -O2 -g -DNDEBUG   -shared -o src\windows\libhidapi.dll -Wl,--out-implib,src\windows\libhidapi.dll.a -Wl,--major-image-version,0,--minor-image-version,14 src/windows/CMakeFiles/hidapi_winapi.dir/hid.c.obj src/windows/CMakeFiles/hidapi_winapi.dir/hidapi_descriptor_reconstruct.c.obj src/windows/CMakeFiles/hidapi_winapi.dir/hidapi.rc.obj  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && cd ."
[6/8] cmd.exe /C "cd . && C:\msys64\mingw64\bin\cc.exe -Wall -Wextra -pedantic -Werror -O2 -g -DNDEBUG  src/windows/test/CMakeFiles/hid_report_reconstructor_test.dir/hid_report_reconstructor_test.c.obj -o src\windows\test\hid_report_reconstructor_test.exe -Wl,--out-implib,src\windows\test\libhid_report_reconstructor_test.dll.a -Wl,--major-image-version,0,--minor-image-version,0  src/windows/libhidapi.dll.a  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && cd ."
[7/8] C:\msys64\mingw64\bin\cc.exe  -IC:/work/hid/hidapi_pr641/hidapi -IC:/work/hid/hidapi_pr641/windows -Wall -Wextra -pedantic -Werror -O2 -g -DNDEBUG -std=gnu11 -MD -MT src/windows/pp_data_dump/CMakeFiles/pp_data_dump.dir/pp_data_dump.c.obj -MF src\windows\pp_data_dump\CMakeFiles\pp_data_dump.dir\pp_data_dump.c.obj.d -o src/windows/pp_data_dump/CMakeFiles/pp_data_dump.dir/pp_data_dump.c.obj -c C:/work/hid/hidapi_pr641/windows/pp_data_dump/pp_data_dump.c
[8/8] cmd.exe /C "cd . && C:\msys64\mingw64\bin\cc.exe -Wall -Wextra -pedantic -Werror -O2 -g -DNDEBUG  src/windows/pp_data_dump/CMakeFiles/pp_data_dump.dir/pp_data_dump.c.obj -o src\windows\pp_data_dump\pp_data_dump.exe -Wl,--out-implib,src\windows\pp_data_dump\libpp_data_dump.dll.a -Wl,--major-image-version,0,--minor-image-version,0  src/windows/libhidapi.dll.a  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && cd ."

mcuee · 2023-11-16T14:40:13Z

On the other hand, I can not reproduce the issue with git main either.

Fix MinGW CI build #632 (comment)

cmorty · 2023-11-16T15:42:00Z

It's a compiler issue:

The C compiler identification is GNU 13.2.0 <- Your system
The C compiler identification is GNU 12.2.0 <- Github-Action

It seems like they turned on -Wformat-signedness by default, which it normally isn't. This MR also turns it on, which shouldn't hurt.

hidtest/test.c

Youw · 2023-11-16T16:05:36Z

windows/hidapi_descriptor_reconstruct.h

+	UINT  CollectionType : 8;
+	UINT  IsAlias : 1;
+	UINT  Reserved : 23;


revert - this may change binary compatibility
any particular reason for this change?

I was very unsure about this one. I'll just drop it. UINT and ULONG should be the same, as int and long are the same. But the only int is covered by the standard (+ implementation defined, of course). When using ULONG I should probably also use %lu instead of %u.

this is a part of reverce-engineered implementation of internal HID structures
unless there is an explicit statement in MS documentation that ULONG is an alias for UINT in all cases - we can't change it

I also allows to revert 98ab22a ( Suppress Visual C++ warning C4214: nonstandard extension used: bit field types other than int) -> https://github.com/cmorty/hidapi/actions/runs/6891832863

my last statement stands

We shouldn't change these, as these are the same types as in the public headers from Microsoft. Using exactly the same is the safest way to keep the binary compatibility.
And binary compatibility of these structs is not covered by our unit tests.

https://godbolt.org/z/5WhYsMW14

lets revert then

Ok, I learned something and hate C/C++ even more. :-( https://godbolt.org/z/9on94frPz
Based on that using UINT seems even more reasonable, but MS's public headers are the killer argument here. I'll drop the commit.

OK. I reassured my pedantic me by adding a static_assert. I hope that solution is fine with you.

libusb/hid.c

windows/hidapi_descriptor_reconstruct.h

ULONG and bitfields are not well defined and behave differently on different compilers. Thus make sure the struct hid_pp_link_collection_node_ has the correct size.

cmorty mentioned this pull request Nov 16, 2023

Disable -Wformat-extra-args form MinGW #635

Closed

cmorty force-pushed the fix-winbuild branch from 57c61cc to fb2b785 Compare November 16, 2023 13:46

mcuee added build system/CI Anything related to building the project or running on CI Windows Related to Windows backend libusb Related to libusb backend labels Nov 16, 2023

Youw reviewed Nov 16, 2023

View reviewed changes

hidtest/test.c Outdated Show resolved Hide resolved

Youw reviewed Nov 16, 2023

View reviewed changes

libusb/hid.c Show resolved Hide resolved

cmorty added 2 commits November 17, 2023 09:26

Fix printf/string-related issues

a888541

CI: Turn on -Wformat-signedness for GNU-cc

b5e803d

cmorty force-pushed the fix-winbuild branch from fb2b785 to b57d3b0 Compare November 17, 2023 08:31

Youw reviewed Nov 17, 2023

View reviewed changes

windows/hidapi_descriptor_reconstruct.h Show resolved Hide resolved

Windows: Assert that struct has the correct size

3e0fbaf

ULONG and bitfields are not well defined and behave differently on different compilers. Thus make sure the struct hid_pp_link_collection_node_ has the correct size.

cmorty force-pushed the fix-winbuild branch from b57d3b0 to 3e0fbaf Compare November 17, 2023 11:50

Youw approved these changes Nov 17, 2023

View reviewed changes

Youw merged commit 4168d87 into libusb:master Nov 17, 2023
11 of 12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the windows build by fixing printf related issues #641

Fix the windows build by fixing printf related issues #641

cmorty commented Nov 16, 2023 •

edited

Loading

mcuee commented Nov 16, 2023 •

edited

Loading

mcuee commented Nov 16, 2023

cmorty commented Nov 16, 2023 •

edited

Loading

Youw Nov 16, 2023

cmorty Nov 16, 2023

Youw Nov 16, 2023

cmorty Nov 16, 2023 •

edited

Loading

Youw Nov 16, 2023

JoergAtGithub Nov 16, 2023

JoergAtGithub Nov 16, 2023

Youw Nov 16, 2023

cmorty Nov 17, 2023

cmorty Nov 17, 2023

Fix the windows build by fixing printf related issues #641

Fix the windows build by fixing printf related issues #641

Conversation

cmorty commented Nov 16, 2023 • edited Loading

mcuee commented Nov 16, 2023 • edited Loading

mcuee commented Nov 16, 2023

cmorty commented Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmorty Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmorty commented Nov 16, 2023 •

edited

Loading

mcuee commented Nov 16, 2023 •

edited

Loading

cmorty commented Nov 16, 2023 •

edited

Loading

cmorty Nov 16, 2023 •

edited

Loading