Skip to content

src: fix use of uninitialized variable - #9281

Closed
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:check-uninitialized
Closed

src: fix use of uninitialized variable#9281
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:check-uninitialized

Conversation

@jasnell

@jasnelljasnell commented Oct 25, 2016

Copy link
Copy Markdown
Member
Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

src

Description of change

Alternative to #9280 ...
The proposed fix in 9280 breaks the calculation algorithm.

Variable was uninitialized in 72547fe Initialize the variable and add a static_check

/cc @bnoordhuis@addaleax

@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 25, 2016
@jasnell

jasnell commented Oct 25, 2016

Copy link
Copy Markdown
MemberAuthor

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

@jasnelljasnell mentioned this pull request Oct 25, 2016
Closed
@jasnell

jasnell commented Oct 28, 2016

Copy link
Copy Markdown
MemberAuthor

ping @addaleax@bnoordhuis

bnoordhuis
bnoordhuis approved these changes Oct 28, 2016
View reviewed changes

@bnoordhuisbnoordhuis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM with nit. Does valgrind -q complain without this patch? If no, consider adding a regression test.

Comment threadsrc/node_i18n.cc Outdated

@bnoordhuisbnoordhuisOct 28, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You have an extraneous paren in the string.

@jasnell
src: fix use of uninitialized variable
e0bad1e
Variable was uninitialized in 72547fe
Initialize the variable and add a static_check
jasnell added a commit that referenced this pull request Oct 28, 2016
@jasnell
src: fix use of uninitialized variable
2bd850b
Variable was uninitialized in 72547fe
Initialize the variable and add a static_check
PR-URL: #9281
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@jasnell

jasnell commented Oct 28, 2016

Copy link
Copy Markdown
MemberAuthor

Landed in 2bd850b

@jasnelljasnell closed this Oct 28, 2016
evanlucas pushed a commit that referenced this pull request Nov 3, 2016
@jasnell@evanlucas
src: fix use of uninitialized variable
5ab172e
Variable was uninitialized in 72547fe
Initialize the variable and add a static_check
PR-URL: #9281
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@evanlucasevanlucas mentioned this pull request Nov 3, 2016
Merged
@MylesBorins

MylesBorins commented Nov 22, 2016

Copy link
Copy Markdown
Contributor

This does not land cleanly in LTS. Added dont-land label. Please feel free to manually backport

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

Reviewers

@bnoordhuisbnoordhuisbnoordhuis 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.

4 participants

@jasnell@MylesBorins@bnoordhuis@nodejs-github-bot