-
Notifications
You must be signed in to change notification settings - Fork 30.7k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
src: pass along errors from --security-reverts
#25466
Conversation
Pass along errors from `Revert()` when a security revert is unknown (which currently applies to all possible values). Previously, we would unconditionally call `exit()`, which is not nice for embedding use cases, and could crash because we were holding a lock for a mutex in `ProcessGlobalArgs()` that would be destroyed by calling `exit()`. Also, add a regression test that makes sure that the process exits with the right exit code and not a crash.
@addaleax sadly an error occured when I tried to trigger a build :( |
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.
LGTM, though I wonder if it is possible to verify this in the option parser...(and have the revertable CVEs displayed somehow? Maybe in —help?)
Fixed the line ending in the test for Windows. |
Re-run of failing node-test-commit-arm |
ARM Rebuild again: https://ci.nodejs.org/job/node-test-commit-arm/21519/ |
Fresh full CI (:crossed_fingers:): https://ci.nodejs.org/job/node-test-pull-request/20246/ |
Landed in 38ab1e9 |
Pass along errors from `Revert()` when a security revert is unknown (which currently applies to all possible values). Previously, we would unconditionally call `exit()`, which is not nice for embedding use cases, and could crash because we were holding a lock for a mutex in `ProcessGlobalArgs()` that would be destroyed by calling `exit()`. Also, add a regression test that makes sure that the process exits with the right exit code and not a crash. PR-URL: #25466 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Pass along errors from `Revert()` when a security revert is unknown (which currently applies to all possible values). Previously, we would unconditionally call `exit()`, which is not nice for embedding use cases, and could crash because we were holding a lock for a mutex in `ProcessGlobalArgs()` that would be destroyed by calling `exit()`. Also, add a regression test that makes sure that the process exits with the right exit code and not a crash. PR-URL: #25466 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Pass along errors from
Revert()
when a security revertis unknown (which currently applies to all possible values).
Previously, we would unconditionally call
exit()
, which isnot nice for embedding use cases, and could crash because we
were holding a lock for a mutex in
ProcessGlobalArgs()
thatwould be destroyed by calling
exit()
.Also, add a regression test that makes sure that the process
exits with the right exit code and not a crash.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes