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

Fixes #39: Adds support for encoding very small and very large integers #40

Merged
merged 3 commits into from
Nov 11, 2015

Conversation

mdouglass
Copy link
Contributor

No description provided.

The validation checks in v0.10 are different than in later versions of
node, so we need to be explicit about converting the high dword to an
integer
…e integers

Integers smaller than Number.MIN_SAFE_INTEGER and larger than
Number.MAX_SAFE_INTEGER are encoded as 8-byte floating point values
@mdouglass
Copy link
Contributor Author

This builds on my previous pull request to support signed int 64s. If you'd prefer, this could be committed separately, but it would leave a gap in the signed integer encoding between the largest 32-bit integer and Number.MAX_SAFE_INTEGER which would fail.

@Enet4
Copy link

Enet4 commented Nov 11, 2015

Well, I admit I hadn't noticed #38 when I posted #39. Thanks for your swift efforts, nevertheless! This fix will definitely suit people better than what we had before. Maybe in the future we could introduce some means of encoding and decoding 64-bit integers without losing precision, but for the moment this patch behaves in a much more predictable way.

@mdouglass
Copy link
Contributor Author

No problem, I actually had the same fix to address on my radar for today. Your issue report just gave me the encouragement to open it sooner.
It would be interesting to link in https://github.com/broofa/node-int64 and optionally use that for decoding 64-bit values.

@mcollina
Copy link
Owner

Superb!

mcollina added a commit that referenced this pull request Nov 11, 2015
Fixes #39: Adds support for encoding very small and very large integers
@mcollina mcollina merged commit d468ceb into mcollina:master Nov 11, 2015
@mdouglass mdouglass deleted the large-numbers branch November 11, 2015 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants