ARROW-1177: [C++] Check for int32 offset overflow in ListBuilder, BinaryBuilder #853
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary, String. Add basic ListArray::FromArrays method, add Python binding Change-Id: I17435c925c070b5e3ca2e76a286c0e810a23d25c
Change-Id: I1271b0b7c13106920fcdf8bd01270795a7739b88
Change-Id: Ide8c2db8d9dd740832c46cdb99a62f8ea21f2fe6
Change-Id: I569f34b7cc108a4078b773eda710605edd32fc6f
Change-Id: Ifdbb610cce9b34c5c3ff9acf9db519fcffacb0da
xhochy
requested changes
Jul 17, 2017
cpp/src/arrow/builder.cc
Outdated
| BinaryBuilder::BinaryBuilder(MemoryPool* pool) : BinaryBuilder(pool, binary()) {} | ||
|
|
||
| Status BinaryBuilder::Init(int64_t elements) { | ||
| DCHECK_LT(elements, std::numeric_limits<int64_t>::max()); |
Member
There was a problem hiding this comment.
This sounds very fishy. Should this be std::numeric_limits<int32_t>::max()?
Member
Author
There was a problem hiding this comment.
Yeah, that seems better. Copy-paste artifact (also fixed this in ListBuilder)
Change-Id: I6b41e5e43004ea716450da5cf9ba95a1af62a19b
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I also refactored BinaryBuilder to not inherit from ListBuilder, which is a bit cleaner. I added a draft of ARROW-507; it needs a unit test and to handle the case where some passed offsets are null (so they need to be sanitized)