-
Notifications
You must be signed in to change notification settings - Fork 28.9k
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
src: add SIGINFO to supported signals #6093
Conversation
@@ -704,6 +704,12 @@ const char *signo_string(int signo) { | |||
# endif | |||
#endif | |||
|
|||
#ifdef SIGINFO | |||
# if SIGINFO != SIGPWR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be # if defined(SIGPWR) && SIGINFO != SIGPWR
- SIGINFO
does not imply the existence of SIGPWR
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That defense is not necessary — the C/C++ preprocessors evaluate all undefined symbols to 0
. See here.
It's worth noting that the code immediately preceding this (Lines 689-705) employs the same bare #if
where the RHS may be undefined. Specific example: SIGPWR is defined on Linux, but not SIGLOST (Source).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That defense is not necessary — the C/C++ preprocessors evaluate all undefined symbols to 0.
It emits nasty warnings, however.
SIGPWR is defined on Linux, but not SIGLOST
Perhaps you mean it's not documented but the define itself exists, it's identical to SIGPWR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to defer to you on this, but I do want to clarify that I gave thought to this initially and determined that Node is not being built with -Wundef
, which would print the nasty warnings you mentioned.
(In fact, V8 relies heavily upon undefined symbols, and adding -Wundef
to the cflags
defined within common.gypi
creates thousands of warnings throughout both the V8 and the Node source.)
I'll update the branch momentarily.
Though not a POSIX signal, SIGINFO is supported by BSD systems (including Mac OS X) and is amongst the few signals that can be triggered in a terminal via a simple key combination (CTRL-T). On Linux, SIGINFO is an alias for SIGPWR; hence the defensive conditionals in src/node.cc.
afc3a44
to
fb97403
Compare
/cc @bnoordhuis |
@bnoordhuis, does my revision (reflected in fb97403) address your concerns? |
LGTM. CI, just in case: https://ci.nodejs.org/job/node-test-pull-request/2251/ |
Thanks @bnoordhuis. The failure looks spurious, but would you please confirm. |
LGTM |
Thanks James, landed in fb5f66a. I'll tag this for v5.x. |
Though not a POSIX signal, SIGINFO is supported by BSD systems (including Mac OS X) and is amongst the few signals that can be triggered in a terminal via a simple key combination (CTRL-T). On Linux, SIGINFO is an alias for SIGPWR; hence the defensive conditionals in src/node.cc. PR-URL: #6093 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Though not a POSIX signal, SIGINFO is supported by BSD systems (including Mac OS X) and is amongst the few signals that can be triggered in a terminal via a simple key combination (CTRL-T). On Linux, SIGINFO is an alias for SIGPWR; hence the defensive conditionals in src/node.cc. PR-URL: #6093 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Buffer: * Buffer.prototype.compare can now compare sub-ranges of two Buffers (James M Snell) #5880 deps: * update to http-parser 2.7.0 (Fedor Indutny) #6279 * update ESLint to 2.7.0 (silverwind) #6132 net: * adds support for passing DNS lookup hints to createConnection() (Colin Ihrig) #6000 node: * Make the builtin libraries available for the --eval and --print CLI options (Anna Henningsen) #6207 npm: * upgrade npm to 3.8.6 (Kat Marchán) #6153 repl: * Pressing enter in the repl will repeat the last command by default if no input has been received. This behaviour was in node previously and was not removed intentionally. (Rich Trott) #6090 src: * add SIGINFO to supported signals (James Reggio) #6093 streams: * Fix a regression that caused by net streams requesting multiple chunks synchronously when combined with cork/uncork (Matteo Collina) #6164 zlib: * The flushing flag is now configurable allowing for decompression of partial data (Anna Henningsen) #6069
Though not a POSIX signal, SIGINFO is supported by BSD systems (including Mac OS X) and is amongst the few signals that can be triggered in a terminal via a simple key combination (CTRL-T). On Linux, SIGINFO is an alias for SIGPWR; hence the defensive conditionals in src/node.cc. PR-URL: #6093 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Buffer: * Buffer.prototype.compare can now compare sub-ranges of two Buffers (James M Snell) #5880 deps: * update to http-parser 2.7.0 (Fedor Indutny) #6279 * update ESLint to 2.7.0 (silverwind) #6132 net: * adds support for passing DNS lookup hints to createConnection() (Colin Ihrig) #6000 node: * Make the builtin libraries available for the --eval and --print CLI options (Anna Henningsen) #6207 npm: * upgrade npm to 3.8.6 (Kat Marchán) #6153 repl: * Pressing enter in the repl will repeat the last command by default if no input has been received. This behavior was in node previously and was not removed intentionally. (Rich Trott) #6090 src: * add SIGINFO to supported signals (James Reggio) #6093 streams: * Fix a regression that caused by net streams requesting multiple chunks synchronously when combined with cork/uncork (Matteo Collina) #6164 zlib: * The flushing flag is now configurable allowing for decompression of partial data (Anna Henningsen) #6069
Buffer: * Buffer.prototype.compare can now compare sub-ranges of two Buffers (James M Snell) #5880 deps: * update to http-parser 2.7.0 (Fedor Indutny) #6279 * update ESLint to 2.7.0 (silverwind) #6132 net: * adds support for passing DNS lookup hints to createConnection() (Colin Ihrig) #6000 node: * Make the builtin libraries available for the --eval and --print CLI options (Anna Henningsen) #6207 npm: * upgrade npm to 3.8.6 (Kat Marchán) #6153 repl: * Pressing enter in the repl will repeat the last command by default if no input has been received. This behaviour was in node previously and was not removed intentionally. (Rich Trott) #6090 src: * add SIGINFO to supported signals (James Reggio) #6093 streams: * Fix a regression that caused by net streams requesting multiple chunks synchronously when combined with cork/uncork (Matteo Collina) #6164 zlib: * The flushing flag is now configurable allowing for decompression of partial data (Anna Henningsen) #6069 PR-URL: #6322
Buffer: * Buffer.prototype.compare can now compare sub-ranges of two Buffers (James M Snell) #5880 deps: * update to http-parser 2.7.0 (Fedor Indutny) #6279 * update ESLint to 2.7.0 (silverwind) #6132 net: * adds support for passing DNS lookup hints to createConnection() (Colin Ihrig) #6000 node: * Make the builtin libraries available for the --eval and --print CLI options (Anna Henningsen) #6207 npm: * upgrade npm to 3.8.6 (Kat Marchán) #6153 repl: * Pressing enter in the repl will repeat the last command by default if no input has been received. This behaviour was in node previously and was not removed intentionally. (Rich Trott) #6090 src: * add SIGINFO to supported signals (James Reggio) #6093 streams: * Fix a regression that caused by net streams requesting multiple chunks synchronously when combined with cork/uncork (Matteo Collina) #6164 zlib: * The flushing flag is now configurable allowing for decompression of partial data (Anna Henningsen) #6069 PR-URL: #6322
Buffer: * Buffer.prototype.compare can now compare sub-ranges of two Buffers (James M Snell) nodejs#5880 deps: * update to http-parser 2.7.0 (Fedor Indutny) nodejs#6279 * update ESLint to 2.7.0 (silverwind) nodejs#6132 net: * adds support for passing DNS lookup hints to createConnection() (Colin Ihrig) nodejs#6000 node: * Make the builtin libraries available for the --eval and --print CLI options (Anna Henningsen) nodejs#6207 npm: * upgrade npm to 3.8.6 (Kat Marchán) nodejs#6153 repl: * Pressing enter in the repl will repeat the last command by default if no input has been received. This behaviour was in node previously and was not removed intentionally. (Rich Trott) nodejs#6090 src: * add SIGINFO to supported signals (James Reggio) nodejs#6093 streams: * Fix a regression that caused by net streams requesting multiple chunks synchronously when combined with cork/uncork (Matteo Collina) nodejs#6164 zlib: * The flushing flag is now configurable allowing for decompression of partial data (Anna Henningsen) nodejs#6069 PR-URL: nodejs#6322
Though not a POSIX signal, SIGINFO is supported by BSD systems (including Mac OS X) and is amongst the few signals that can be triggered in a terminal via a simple key combination (CTRL-T). On Linux, SIGINFO is an alias for SIGPWR; hence the defensive conditionals in src/node.cc. PR-URL: #6093 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Buffer: * Buffer.prototype.compare can now compare sub-ranges of two Buffers (James M Snell) #5880 deps: * update to http-parser 2.7.0 (Fedor Indutny) #6279 * update ESLint to 2.7.0 (silverwind) #6132 net: * adds support for passing DNS lookup hints to createConnection() (Colin Ihrig) #6000 node: * Make the builtin libraries available for the --eval and --print CLI options (Anna Henningsen) #6207 npm: * upgrade npm to 3.8.6 (Kat Marchán) #6153 repl: * Pressing enter in the repl will repeat the last command by default if no input has been received. This behaviour was in node previously and was not removed intentionally. (Rich Trott) #6090 src: * add SIGINFO to supported signals (James Reggio) #6093 streams: * Fix a regression that caused by net streams requesting multiple chunks synchronously when combined with cork/uncork (Matteo Collina) #6164 zlib: * The flushing flag is now configurable allowing for decompression of partial data (Anna Henningsen) #6069 PR-URL: #6322
Checklist
Affected core subsystem(s)
Exposed through
process
module, change inconstants
withinsrc
.Description of change
Though not a POSIX signal, SIGINFO is supported by BSD systems (including Mac OS X) and is amongst the few signals that can be triggered in a terminal via a simple key combination (CTRL-T).
Supporting SIGINFO on these systems will enable the use case where long-running terminal applications can report their status upon receiving the signal, and the signal can be triggered with a keystroke, instead of requiring the use of
kill
.A brief examination of
src/node_constants.cc
indicates that there's no philosophical decision to support only POSIX signals, so I assume SIGINFO was an oversight. (SIGPWR, SIGLOST, and SIGWINCH are all present and non-POSIX). It's worth noting that on Linux, SIGINFO is an alias for SIGPWR, which is why I employed defensive conditionals insrc/node.cc
.