Skip to content

Conversation

@bnoordhuis
Copy link
Member

#17372 with updates, cc @danbev.

Daniel indicated he was short on time and I figured we could go above and beyond the suggestion from #17372 (comment).

danbevand others added 2 commits March 2, 2018 16:57
PR-URL: nodejs#17372 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Convert manual memory management of STACK_OF(X509) instances to std::unique_ptr with a custom deleter. Note the tasteful application of std::move() to indicate a function that consumes its argument.
@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. labels Mar 2, 2018
@bnoordhuisbnoordhuis mentioned this pull request Mar 2, 2018
2 tasks
@BridgeAR
Copy link
Member

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 2, 2018
@danbev
Copy link
Contributor

node-test-commit failures looks unrelated

console output:

Makefile:90: recipe for target 'node' failedmake[1]: *** [node] Error 2Makefile:464: recipe for target 'build-ci' failedmake: *** [build-ci] Error 2Build step 'Conditional steps (multiple)' marked build as failureRun condition [Always] enabling perform for step [[]]TAP Reports Processing: STARTLooking for TAP results report in workspace using pattern: *.tapDid not find any matching files. Setting build result to FAILURE.Checking ^not okJenkins Text Finder: File set '*.tap' is emptyNotifying upstream projects of job completionFinished: FAILURE
node-test-commit-linuxone failure looks unrelated

console output:

not ok 864 parallel/test-http2-https-fallback --- duration_ms: 0.263 severity: fail stack: |- (node:49136) ExperimentalWarning: The http2 module is an experimental API. assert.js:243 throw err; ^ AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value: assert.ok(/Unknown ALPN Protocol, expected `h2` to be available/.test(text)) at TLSSocket.tls.setEncoding.on.on.common.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-http2-https-fallback.js:143:9) at TLSSocket.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:467:15) at TLSSocket.emit (events.js:136:15) at endReadableNT (_stream_readable.js:1021:12) at process._tickCallback (internal/process/next_tick.js:115:19)
node-test-commit-windows-fanned failures looks unrelated

console output:

c:\workspace\node-test-binary-windows>git reset --hard || trueHEAD is now at 1cef52e814 added binariesc:\workspace\node-test-binary-windows>mv ../test.tap test.tap || truemv: cannot stat '../test.tap': No such file or directoryc:\workspace\node-test-binary-windows>mv ../cctest.tap cctest.tap || truemv: cannot stat '../cctest.tap': No such file or directoryc:\workspace\node-test-binary-windows>exit /b 1 Build step 'Execute Windows batch command' marked build as failureTAP Reports Processing: STARTLooking for TAP results report in workspace using pattern: *.tapDid not find any matching files. Setting build result to FAILURE.Checking ^not okJenkins Text Finder: File set '*.tap' is emptyNotifying upstream projects of job completionFinished: FAILURE

danbev added a commit that referenced this pull request Mar 6, 2018
PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
danbev pushed a commit that referenced this pull request Mar 6, 2018
Convert manual memory management of STACK_OF(X509) instances to std::unique_ptr with a custom deleter. Note the tasteful application of std::move() to indicate a function that consumes its argument. PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
@danbev
Copy link
Contributor

Landed as 0c7e7d4, and e46d0bc. Thanks for all the help/reviews!

@danbevdanbev closed this Mar 6, 2018
MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
Convert manual memory management of STACK_OF(X509) instances to std::unique_ptr with a custom deleter. Note the tasteful application of std::move() to indicate a function that consumes its argument. PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Mar 6, 2018
MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
Convert manual memory management of STACK_OF(X509) instances to std::unique_ptr with a custom deleter. Note the tasteful application of std::move() to indicate a function that consumes its argument. PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
Convert manual memory management of STACK_OF(X509) instances to std::unique_ptr with a custom deleter. Note the tasteful application of std::move() to indicate a function that consumes its argument. PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
Convert manual memory management of STACK_OF(X509) instances to std::unique_ptr with a custom deleter. Note the tasteful application of std::move() to indicate a function that consumes its argument. PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 7, 2018
PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 7, 2018
Convert manual memory management of STACK_OF(X509) instances to std::unique_ptr with a custom deleter. Note the tasteful application of std::move() to indicate a function that consumes its argument. PR-URL: #19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
PR-URL: nodejs#19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Convert manual memory management of STACK_OF(X509) instances to std::unique_ptr with a custom deleter. Note the tasteful application of std::move() to indicate a function that consumes its argument. PR-URL: nodejs#19087 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
@jasnell
Copy link
Member

Should this be backported to 8.x? If so, we need a separate backport PR.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.cryptoIssues and PRs related to the crypto subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@bnoordhuis@BridgeAR@danbev@jasnell@addaleax@nodejs-github-bot