Skip to content

Conversation

@bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Oct 19, 2016

@bnoordhuis bnoordhuis added c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol labels Oct 19, 2016
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

Could you replace the if (err) throw err; in the tests with assert.ifError(err);. Other than that, LGTM.

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@ofrobots ofrobots left a comment

Choose a reason for hiding this comment

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

LGTM.

@bnoordhuis
Copy link
Member Author

bnoordhuis commented Oct 24, 2016

@bnoordhuis bnoordhuis force-pushed the fix-inspector-nullptr branch from d3e654e to 7c933a7 Compare October 24, 2016 20:28
@bnoordhuis
Copy link
Member Author

Flakes on a freebsd and smartos buildbot, an infrastructure issue on one of the rpi1 machines. Nothing that is caused by this pull request so I'll go ahead and land it.

Fix a nullptr dereference when an invalid path is requested.

Regression introduced in commit 69fc85d ("inspector: generate UUID for
debug targets"), caught by Coverity.

PR-URL: nodejs#9184
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Remove parallel/test-v8-inspector-json-protocol, it duplicates the test
found in inspector/test-inspector.

PR-URL: nodejs#9184
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@bnoordhuis bnoordhuis force-pushed the fix-inspector-nullptr branch from 7c933a7 to 8e54c96 Compare October 24, 2016 21:08
@bnoordhuis bnoordhuis closed this Oct 24, 2016
@bnoordhuis bnoordhuis deleted the fix-inspector-nullptr branch October 24, 2016 21:08
@bnoordhuis bnoordhuis merged commit 8e54c96 into nodejs:master Oct 24, 2016
evanlucas pushed a commit that referenced this pull request Nov 2, 2016
Fix a nullptr dereference when an invalid path is requested.

Regression introduced in commit 69fc85d ("inspector: generate UUID for
debug targets"), caught by Coverity.

PR-URL: #9184
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Nov 2, 2016
Remove parallel/test-v8-inspector-json-protocol, it duplicates the test
found in inspector/test-inspector.

PR-URL: #9184
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 18, 2016
Fix a nullptr dereference when an invalid path is requested.

Regression introduced in commit 69fc85d ("inspector: generate UUID for
debug targets"), caught by Coverity.

PR-URL: #9184
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 18, 2016
Remove parallel/test-v8-inspector-json-protocol, it duplicates the test
found in inspector/test-inspector.

PR-URL: #9184
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 19, 2016
Fix a nullptr dereference when an invalid path is requested.

Regression introduced in commit 69fc85d ("inspector: generate UUID for
debug targets"), caught by Coverity.

PR-URL: #9184
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 19, 2016
Remove parallel/test-v8-inspector-json-protocol, it duplicates the test
found in inspector/test-inspector.

PR-URL: #9184
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Nov 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants