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.

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

src: use std::string for trace enabled_categories #12242

Conversation

sam-github
Copy link
Contributor

A std::string manages its own memory, so using one removes the implicit
assumption that the argv vector passed to node will never be
deallocated. Also, the enabled_categories are used to construct a
std::stringstream, so its simpler to use the standard library
consistently.

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

src

A std::string manages its own memory, so using one removes the  implicit
assumption that the argv vector passed to node will never be
deallocated. Also, the enabled_categories are used to construct a
std::stringstream, so its simpler to use the standard library
consistently.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events. labels Apr 5, 2017
@sam-github
Copy link
Contributor Author

@addaleax
Copy link
Member

addaleax commented Apr 9, 2017

Landed in 809ca2f

@addaleax addaleax closed this Apr 9, 2017
addaleax pushed a commit that referenced this pull request Apr 9, 2017
A std::string manages its own memory, so using one removes the implicit
assumption that the argv vector passed to node will never be
deallocated. Also, the enabled_categories are used to construct a
std::stringstream, so its simpler to use the standard library
consistently.

PR-URL: #12242
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Apr 10, 2017
A std::string manages its own memory, so using one removes the implicit
assumption that the argv vector passed to node will never be
deallocated. Also, the enabled_categories are used to construct a
std::stringstream, so its simpler to use the standard library
consistently.

PR-URL: nodejs#12242
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
@italoacasas italoacasas mentioned this pull request Apr 10, 2017
2 tasks
@sam-github sam-github deleted the use-std-string-for-trace-categories branch April 17, 2017 20:24
@MylesBorins
Copy link
Contributor

should this be backported?

@sam-github
Copy link
Contributor Author

@MylesBorins it is a refactor, but not backporting will make conflicts with subsequent PRs more likely, I'll backport

@sam-github
Copy link
Contributor Author

Trace feature doesn't exist on 6.x, removed the backport requested.

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++. trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants