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

src: use unique pointer for tracing_agent #17012

Closed
wants to merge 1 commit into from
Closed

src: use unique pointer for tracing_agent #17012

wants to merge 1 commit into from

Conversation

fhinkel
Copy link
Member

@fhinkel fhinkel commented Nov 14, 2017

Use std::unique_ptr instead of raw pointers for the
tracing_agent_ in node.cc. This makes ownership clearer and we
don't risk a memory leak.

Checklist
Affected core subsystem(s)

src

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 14, 2017
@fhinkel
src: use unique pointer for tracing_agent
0f96d04 
Use std::unique_ptr instead of raw pointers for the
tracing_agent_ in node.cc. This makes ownership clearer and we
don't risk a memory leak.
@fhinkel fhinkel force-pushed the nov/uniq_ptr_tracing_agent branch from 6d046bb to 0f96d04 Compare November 14, 2017 09:04
@fhinkel fhinkel mentioned this pull request Nov 15, 2017
Closed
2 tasks
@fhinkel
Copy link
Member Author

fhinkel commented Nov 15, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/11460/

@fhinkel
Copy link
Member Author

fhinkel commented Nov 16, 2017

Landed in 787863d.

@fhinkel fhinkel closed this Nov 16, 2017
fhinkel added a commit that referenced this pull request Nov 16, 2017
@fhinkel
src: use unique pointer for tracing_agent
787863d 
Use std::unique_ptr instead of raw pointers for the
tracing_agent_ in node.cc. This makes ownership clearer and we
don't risk a memory leak.

PR-URL: #17012
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
fhinkel added a commit that referenced this pull request Nov 17, 2017
@fhinkel
doc: mention smart pointers in Cpp style guide
0e1abb1 
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 11, 2017
@fhinkel @MylesBorins
src: use unique pointer for tracing_agent
98a0770 
Use std::unique_ptr instead of raw pointers for the
tracing_agent_ in node.cc. This makes ownership clearer and we
don't risk a memory leak.

PR-URL: #17012
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@fhinkel @MylesBorins
doc: mention smart pointers in Cpp style guide
03803ee 
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
Merged
@gibfahn
Copy link
Member

gibfahn commented Dec 19, 2017 •
edited by MylesBorins
Loading

Should this be backported to v8.x-staging || v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

Doesn't land cleanly on v8.x.

gibfahn pushed a commit that referenced this pull request Dec 19, 2017
@fhinkel @gibfahn
doc: mention smart pointers in Cpp style guide
f30fcdb 
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@fhinkel @gibfahn
doc: mention smart pointers in Cpp style guide
706aab9 
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@AndreasMadsen AndreasMadsen mentioned this pull request Jan 16, 2018
Closed
@AndreasMadsen AndreasMadsen mentioned this pull request Jan 16, 2018
Closed
4 tasks
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 17, 2018
@fhinkel @gibfahn
src: use unique pointer for tracing_agent
8a3da7a 
Use std::unique_ptr instead of raw pointers for the
tracing_agent_ in node.cc. This makes ownership clearer and we
don't risk a memory leak.

PR-URL: nodejs#17012
Backport-PR-URL: nodejs#18179
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@fhinkel @MylesBorins
src: use unique pointer for tracing_agent
ace2c2f 
Use std::unique_ptr instead of raw pointers for the
tracing_agent_ in node.cc. This makes ownership clearer and we
don't risk a memory leak.

Backport-PR-URL: #18179
PR-URL: #17012
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
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

@TimothyGu TimothyGu TimothyGu approved these changes

@jasnell jasnell jasnell approved these changes

@danbev danbev danbev approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@fhinkel @gibfahn @danbev @jasnell @TimothyGu @cjihrig @MylesBorins @BethGriggs @nodejs-github-bot