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

Windows: small fixes and fewer warnings #261

Merged
merged 5 commits into from
Jan 8, 2015

Conversation

piscisaureus
Copy link
Contributor

No description provided.

This patch disables two (categories of) warnings:

  * deprecation of GetVersionExA
  * possible loss of data in implicit conversion of scalar types

These warnings don't seem to point out serious problems, and avoiding
them in openssl is somebody else's business.

PR-URL: nodejs#261
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
For consistency with the rest of the source code, use the wide-char
version of this API.

PR-URL: nodejs#261
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
There is no other way to retrieve the Windows version. The stated reason
this API is deprecated is that applications are not supposed to check
whether the Windows it's running on is recent enough. But that's not
what we use it for.

PR-URL: nodejs#261
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The NODE_NET_SOCKET_READ and NODE_NET_SOCKET_WRITE macros are just
no-ops on Windows, but they used to be defined as taking four parameters
while being called with five arguments. Turn them into variadic macros
to squelch a compiler warning.

PR-URL: nodejs#261
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The PLATFORM preprocessor symbol is defined in node.gyp, and on Windows
it's set to "win". This conflicts with a built-in preprocessor symbol
with a different value ("win32"), which makes the linker(!) complain.
Resolve this by renaming these symbols to NODE_ARCH and NODE_PLATFORM.

PR-URL: nodejs#261
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@piscisaureus piscisaureus merged commit e1fe270 into nodejs:v1.x Jan 8, 2015
@piscisaureus piscisaureus deleted the no-warnings branch January 8, 2015 13:18
@bnoordhuis
Copy link
Member

I didn't actually LGTM it, Bertje.

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