Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sc_sock: fix compiler warnings for Windows #127

Merged
merged 1 commit into from
Dec 20, 2023
Merged

Conversation

svladykin
Copy link
Contributor

  • Fix types
  • Use pragma only for MSVC
  • Drop C11 anonymous union
  • Use INVALID_SOCKET instead of SOCKET_ERROR where appropriate

Copy link

codecov bot commented Dec 19, 2023

Codecov Report

Merging #127 (e0188e7) into master (e7134b7) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #127   +/-   ##
=======================================
  Coverage   99.81%   99.81%           
=======================================
  Files          21       21           
  Lines        2195     2195           
  Branches      388      388           
=======================================
  Hits         2191     2191           
  Misses          1        1           
  Partials        3        3           
Files Coverage Δ
socket/sc_sock.c 99.19% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7134b7...e0188e7. Read the comment docs.

@tezc
Copy link
Owner

tezc commented Dec 19, 2023

Hey @svladykin, hope you are doing well! Any idea why we don't see these warnings on CI? Maybe we need to compile release version to see it?

@svladykin
Copy link
Contributor Author

I guess this is because we don't have any of these flags enabled for Windows:

if (NOT CMAKE_C_COMPILER_ID MATCHES "MSVC")
    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -Wall -Wextra -pedantic -pthread -Werror")
endif ()

Not sure if they will actually work for MSVC though.

I experimented with cross-compilation for Windows on Linux:

  • tried Zig CC (uses Clang inside) and it can compile the code but fails to link, but this is enough to produce all the warnings;
  • was able to successfully cross-compile a dll with Mingw (uses Gcc inside).

This PR is based on Mingw and Zig CC output.

@tezc
Copy link
Owner

tezc commented Dec 20, 2023

Ok, I'll take a look that later.

@tezc tezc merged commit c729344 into tezc:master Dec 20, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants