Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

process: use uv_signal instead of ev_signal #3890

Closed
wants to merge 2 commits into from

Conversation

piscisaureus
Copy link

Since libev is on it's way out, ev_signal will be removed, so switch to
the libuv equivalent. As a bonus, this sort of works on Windows too.

@piscisaureus
Copy link
Author

TBR by @isaacs, @bnoordhuis, @indutny or @TooTallNate

@piscisaureus
Copy link
Author

After landing this we could possible remove the old node_signal_watcher.*. I would have no objections.

Since libev is on it's way out, ev_signal will be removed, so switch to
the libuv equivalent. As a bonus, this sort of works on Windows too.
assert(args.IsConstructCall());

HandleScope scope;
SignalWrap *wrap = new SignalWrap(args.This());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SignalWrap* wrap = ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I copied this from timer_wrap :-). Will change.

@bnoordhuis
Copy link
Member

Couple of nits, otherwise LGTM.

@piscisaureus
Copy link
Author

Landed in 600a646.

richardlau pushed a commit to ibmruntimes/node that referenced this pull request Dec 14, 2015
PR nodejs#3890 [1] introduced the variable ALLOW_INSECURE_SERVER_DHPARAM defined
in src/node_crypto.cc. However, if nodejs is built without OpenSSL support,
the build fails:
 error: ‘ALLOW_INSECURE_SERVER_DHPARAM’ was not declared in this scope
       ALLOW_INSECURE_SERVER_DHPARAM = true;

Fix this by using the preprocessor macro HAVE_OPENSSL to opt-out the use of
ALLOW_INSECURE_SERVER_DHPARAM in non-OpenSSL builds.

[1] nodejs/node#3890

PR-URL: nodejs/node#4201
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <rod@vagg.org>
richardlau pushed a commit to ibmruntimes/node that referenced this pull request Feb 15, 2016
PR nodejs#3890 [1] introduced the variable ALLOW_INSECURE_SERVER_DHPARAM defined
in src/node_crypto.cc. However, if nodejs is built without OpenSSL support,
the build fails:
 error: ‘ALLOW_INSECURE_SERVER_DHPARAM’ was not declared in this scope
       ALLOW_INSECURE_SERVER_DHPARAM = true;

Fix this by using the preprocessor macro HAVE_OPENSSL to opt-out the use of
ALLOW_INSECURE_SERVER_DHPARAM in non-OpenSSL builds.

[1] nodejs/node#3890

PR-URL: nodejs/node#4201
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <rod@vagg.org>
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
PR nodejs#3890 [1] introduced the variable ALLOW_INSECURE_SERVER_DHPARAM defined
in src/node_crypto.cc. However, if nodejs is built without OpenSSL support,
the build fails:
 error: ‘ALLOW_INSECURE_SERVER_DHPARAM’ was not declared in this scope
       ALLOW_INSECURE_SERVER_DHPARAM = true;

Fix this by using the preprocessor macro HAVE_OPENSSL to opt-out the use of
ALLOW_INSECURE_SERVER_DHPARAM in non-OpenSSL builds.

[1] nodejs/node#3890

PR-URL: nodejs/node#4201
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <rod@vagg.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants