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

server.cpp: Fix comparison between signed and unsigned values #2795

Conversation

davidebeatrici
Copy link
Member

@davidebeatrici davidebeatrici commented Jan 31, 2017

https://msdn.microsoft.com/it-it/library/windows/desktop/ms740516(v=vs.85).aspx

Because the SOCKET type is unsigned, MinGW triggers a warning:

warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

This commit compares the socket to "INVALID_SOCKET" instead of "-1", as recommended by MSDN.

https://msdn.microsoft.com/it-it/library/windows/desktop/ms740516(v=vs.85).aspx

Because the SOCKET type is unsigned, MinGW triggers a warning:
warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

This commit compares the socket to "INVALID_SOCKET" instead of "-1", as recommended by MSDN.
@davidebeatrici davidebeatrici added the build Everything related to compiling/building the code label Jan 31, 2017
@mkrautz
Copy link
Contributor

mkrautz commented Feb 1, 2017

I think I would prefer if we just expanded whole function into two separate paths: one for Unix, one for Windows. It's already ifdef-soup.

Let me submit a counter-proposal.

@mkrautz
Copy link
Contributor

mkrautz commented Feb 1, 2017

See #2801

WDYT?

@davidebeatrici
Copy link
Member Author

Reviewed, LGTM.

@Kissaki
Copy link
Member

Kissaki commented Feb 2, 2017

Closing in favor of #2801

@Kissaki Kissaki closed this Feb 2, 2017
@Kissaki
Copy link
Member

Kissaki commented Feb 2, 2017

@davidebeatrici that comment is pretty confusing here. First time I read it I understood that it references another references and not this PR you commented on (even though you can comment over there). But just now I close this, and then got shocked whether I was in the wrong PR. But I was not. ;-)

@davidebeatrici
Copy link
Member Author

@Kissaki Sorry, I should have specified that I was talking about mkrautz's PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Everything related to compiling/building the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants