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

Fix bug in IsLong implementation #4655

Merged
merged 1 commit into from
Apr 22, 2020
Merged

Fix bug in IsLong implementation #4655

merged 1 commit into from
Apr 22, 2020

Conversation

russcam
Copy link
Contributor

@russcam russcam commented Apr 22, 2020

Relates: #4649

This commit fixes a bug in the IsLong implementation, and prefers
reading long values over double values in PrimitiveObjectFormatter
when the bytes represent a valid long value, to avoid floating
point rounding errors that may happen when parsing a double from
a long value.

The bug in IsLong was that the loop over the array segment bytes was
exiting early when the count of bytes was the same as long.Min/MaxValue
length, and the current loop byte is less than the byte at index in
long.Min/MaxValue. In this scenario, it should not exit early, but
not continue to check following loop bytes against byte at index in
long.Min/MaxValue. Following loop iterations still need to check
whether remaining bytes are numbers.

Relates: #4649

This commit fixes a bug in the IsLong implementation, and prefers
reading long values over double values in PrimitiveObjectFormatter
when the bytes represent a valid long value, to avoid floating
point rounding errors that may happen when parsing a double from
a long value.

The bug in IsLong was that the loop over the array segment bytes was
exiting early when the count of bytes was the same as long.Min/MaxValue
length, and the current loop byte is less than the byte at index in
long.Min/MaxValue. In this scenario, it should not exit early, but
not continue to check following loop bytes against byte at index in
long.Min/MaxValue. Following loop iterations still need to check
whether remaining bytes are numbers.
Copy link
Member

@Mpdreamz Mpdreamz left a comment

Choose a reason for hiding this comment

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

Much better fix then my initial patch 👍

@Mpdreamz
Copy link
Member

Failing tests are:

@Mpdreamz Mpdreamz merged commit f8c6940 into master Apr 22, 2020
@Mpdreamz Mpdreamz deleted the fix/is-long branch April 22, 2020 07:30
github-actions bot pushed a commit that referenced this pull request Apr 22, 2020
Relates: #4649

This commit fixes a bug in the IsLong implementation, and prefers
reading long values over double values in PrimitiveObjectFormatter
when the bytes represent a valid long value, to avoid floating
point rounding errors that may happen when parsing a double from
a long value.

The bug in IsLong was that the loop over the array segment bytes was
exiting early when the count of bytes was the same as long.Min/MaxValue
length, and the current loop byte is less than the byte at index in
long.Min/MaxValue. In this scenario, it should not exit early, but
not continue to check following loop bytes against byte at index in
long.Min/MaxValue. Following loop iterations still need to check
whether remaining bytes are numbers.
github-actions bot pushed a commit that referenced this pull request Apr 22, 2020
Relates: #4649

This commit fixes a bug in the IsLong implementation, and prefers
reading long values over double values in PrimitiveObjectFormatter
when the bytes represent a valid long value, to avoid floating
point rounding errors that may happen when parsing a double from
a long value.

The bug in IsLong was that the loop over the array segment bytes was
exiting early when the count of bytes was the same as long.Min/MaxValue
length, and the current loop byte is less than the byte at index in
long.Min/MaxValue. In this scenario, it should not exit early, but
not continue to check following loop bytes against byte at index in
long.Min/MaxValue. Following loop iterations still need to check
whether remaining bytes are numbers.
russcam added a commit that referenced this pull request May 7, 2020
Relates: #4649

This commit fixes a bug in the IsLong implementation, and prefers
reading long values over double values in PrimitiveObjectFormatter
when the bytes represent a valid long value, to avoid floating
point rounding errors that may happen when parsing a double from
a long value.

The bug in IsLong was that the loop over the array segment bytes was
exiting early when the count of bytes was the same as long.Min/MaxValue
length, and the current loop byte is less than the byte at index in
long.Min/MaxValue. In this scenario, it should not exit early, but
not continue to check following loop bytes against byte at index in
long.Min/MaxValue. Following loop iterations still need to check
whether remaining bytes are numbers.

(cherry picked from commit f8c6940)
@russcam russcam added v7.6.2 and removed v7.7.0 labels May 7, 2020
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.

2 participants