Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

src: fix bad sizeof expression #17014

Merged
merged 1 commit into from
Nov 15, 2017
Merged

Conversation

bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Nov 14, 2017

It was computing the size of the pointer, not the size of the pointed-to
object.

Introduced in commit 727b291 ("src,dns: refactor cares_wrap to avoid
global state".)

CI: https://ci.nodejs.org/job/node-test-pull-request/11416/ (jenkins issue)
CI: https://ci.nodejs.org/job/node-test-pull-request/11418/

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. labels Nov 14, 2017
It was computing the size of the pointer, not the size of the pointed-to
object.

Introduced in commit 727b291 ("src,dns: refactor cares_wrap to avoid
global state".)

PR-URL: nodejs#17014
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@bnoordhuis bnoordhuis closed this Nov 15, 2017
@bnoordhuis bnoordhuis deleted the fix-cares-sizeof branch November 15, 2017 11:12
@bnoordhuis bnoordhuis merged commit 8203ce8 into nodejs:master Nov 15, 2017
@bnoordhuis
Copy link
Member Author

I've landed this in 8203ce8 in < 48 hours because it's an out and out bug, not much discussion possible (but plenty of sign-offs, for that matter.)

MylesBorins pushed a commit that referenced this pull request Dec 11, 2017
It was computing the size of the pointer, not the size of the pointed-to
object.

Introduced in commit 727b291 ("src,dns: refactor cares_wrap to avoid
global state".)

PR-URL: #17014
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
@gibfahn
Copy link
Member

gibfahn commented Dec 13, 2017

Fix for #14518, so should land with that on v6.x (if it goes back).

gibfahn pushed a commit that referenced this pull request Dec 13, 2017
It was computing the size of the pointer, not the size of the pointed-to
object.

Introduced in commit 727b291 ("src,dns: refactor cares_wrap to avoid
global state".)

PR-URL: #17014
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
This was referenced Dec 20, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding.
Projects
None yet
Development

Successfully merging this pull request may close these issues.