Skip to content
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.

Sign up for GitHub

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

Jump to bottom

inspector: fix Coverity defects #10240

Merged
merged 0 commits into from
Dec 14, 2016
Merged

inspector: fix Coverity defects #10240

merged 0 commits into from
Dec 14, 2016

Conversation

eugeneo
Copy link
Contributor

@eugeneo eugeneo commented Dec 12, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

Inspector: just minor fixes to address Coverity comments

Description of change

One defect remains - Coverity believes that a session object is never
freed while in reality its lifespan is tied to a libuv socket.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v4.x inspector Issues and PRs related to the V8 inspector protocol labels Dec 12, 2016
@eugeneo eugeneo requested a review from ofrobots December 12, 2016 18:54
@eugeneo eugeneo closed this Dec 14, 2016
@eugeneo eugeneo deleted the coverty-sserver branch December 14, 2016 17:58
@eugeneo
Copy link
Contributor Author

eugeneo commented Dec 14, 2016

Landed as 3c29b2f

@eugeneo eugeneo merged commit 3c29b2f into nodejs:master Dec 14, 2016
@MylesBorins
Copy link
Contributor

@Fishrock123 not sure how the bot came up with these labels... this one definitely does not land cleanly on any branch

@MylesBorins
Copy link
Contributor

@eugeneo this does not land cleanly on v7.x-staging

It appears there are some paths on master that don't exist on v7.x

* Unmerged path src/inspector_socket_server.cc
* Unmerged path test/cctest/test_inspector_socket_server.cc

@joyeecheung joyeecheung mentioned this pull request Dec 16, 2016
Closed
2 tasks
@MylesBorins MylesBorins mentioned this pull request Dec 17, 2016
Merged
@evanlucas evanlucas mentioned this pull request Jan 3, 2017
Merged
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
@evanlucas
inspector: fix Coverity defects
f1a1789 
One defect remains - Coverity believes that a session object is never
freed while in reality its lifespan is tied to a libuv socket.

PR-URL: #10240
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
@evanlucas
inspector: fix Coverity defects
4bed947 
One defect remains - Coverity believes that a session object is never
freed while in reality its lifespan is tied to a libuv socket.

PR-URL: #10240
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax addaleax approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@ofrobots ofrobots ofrobots approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@eugeneo @MylesBorins @ofrobots @bnoordhuis @addaleax @cjihrig @nodejs-github-bot