Skip to content

Conversation

@trevnorris
Copy link
Contributor

@trevnorris trevnorris commented Jun 1, 2016

  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

async_wrap

Description of change

Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

No test was included because the only way to increase the uid is to create new handles, and creating more than a uin32_t's worth of handles would take a while.

R=@bnoordhuis
R=@AndreasMadsen

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

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 1, 2016
@trevnorris trevnorris mentioned this pull request Jun 1, 2016
3 tasks
@addaleax
Copy link
Member

addaleax commented Jun 1, 2016

LGTM

2 similar comments
@cjihrig
Copy link
Contributor

cjihrig commented Jun 1, 2016

LGTM

@AndreasMadsen
Copy link
Member

AndreasMadsen commented Jun 1, 2016

LGTM

Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

PR-URL: nodejs#7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
@trevnorris trevnorris force-pushed the asyncwrap-uid-to-double branch from c686a88 to f81f0c3 Compare June 1, 2016 21:40
@trevnorris trevnorris closed this Jun 1, 2016
@trevnorris trevnorris deleted the asyncwrap-uid-to-double branch June 1, 2016 21:41
@trevnorris trevnorris merged commit f81f0c3 into nodejs:master Jun 1, 2016
@trevnorris
Copy link
Contributor Author

Thanks much! Landed in f81f0c3.

Fishrock123 pushed a commit that referenced this pull request Jun 1, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 7, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

Ref: #7048
PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

Ref: #7048
PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 28, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

Ref: #7048
PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

Ref: #7048
PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
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++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants