Skip to content

Conversation

@johnynek
Copy link
Collaborator

we hit this with a 70MB string. Work around by removing a byte. Better to fix.

val dec = decBuf._1
val buf = decBuf._2
val maxSpace = (b.length * dec.maxCharsPerByte).toInt + 1
val maxSpace = (b.length * (dec.maxCharsPerByte.toDouble)).toInt + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd put a comment, that for cases where it's possible to have 2B, just try using 2B. I'd also drop the () and maybe just toDouble on the length, since that's the thing that can be large.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

actually, the bug is that dec.maxCharsPerByte returns a Float. That can only store all integers up to 2^{24}. After that it steps up by 2. So the toInt truncates by 2 in that case, not rounding down by one.

We need to convert that Float to a Double before the *.

I don't know what 2B means in this case. Can you explain?

Copy link
Contributor

Choose a reason for hiding this comment

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

2B = 2 billion (1 << 31), since that's how toInt handles larger values.

But yeah, Float -> Double makes more sense than integral -> Double, so that seems fine.

ianoc added a commit that referenced this pull request Feb 16, 2016
Fix an integral float overflow for very large strings
@ianoc ianoc merged commit 73de35f into develop Feb 16, 2016
@ianoc ianoc deleted the oscar/fix-float-overflow branch February 16, 2016 19:45
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.

5 participants