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

WIN32: the nfds parameter of select() is unused. #147

Merged
merged 1 commit into from
Dec 3, 2023

Conversation

carlo-bramini
Copy link
Contributor

Winsock provides an opaque SOCKET type for handling sockets and the nfds parameter of select() is ignored but kept for compatibity with BSD standard.
Expecting to handle sockets by not assuming to be an int type, this could cause a complain from MSVC when compiling for 64bit for Windows because incompatible types, so it would be better to just ignore this and just leave nfds to zero for the future.
See also here:
https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-select

immagine

@JohannesLorenz
Copy link
Collaborator

Thanks for the PR, though there are 2 issues:

  • CI fails (even for Linux) - maybe we need convert !defined into not defined or use #ifndef?
  • Is it wanted that the 3rd select call in server.c is not changed by this PR?

Winsock provides an opaque SOCKET type for handling sockets and the nfds parameter of select() is ignored but kept for compatibity with BSD standard.
Expecting to handle sockets by not assuming to be an int type, this could cause a complain from MSVC when compiling for 64bit for Windows because incompatible types, so it would be better to just ignore this and just leave nfds to zero.
@carlo-bramini
Copy link
Contributor Author

I did the changes that you suggested to me, thank you.
Now the CI seems to be fine, except the MAC test that it still hangs, but it doesn't seem to be related to this PR.

@JohannesLorenz
Copy link
Collaborator

Yes, we still need to work on the macOS CI 😁 Thanks for fixing, I will merge that.

@JohannesLorenz JohannesLorenz merged commit ed21c61 into radarsat1:master Dec 3, 2023
2 of 3 checks passed
@carlo-bramini carlo-bramini deleted the fix_windows_2 branch December 3, 2023 11:58
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