pitrou commented on code in PR #49883:
URL: https://github.com/apache/arrow/pull/49883#discussion_r3201531889
##########
cpp/src/arrow/buffer_builder.h:
##########
@@ -74,6 +79,12 @@ class ARROW_EXPORT BufferBuilder {
/// shrinking the builder.
/// \return Status
Status Resize(const int64_t new_capacity, bool shrink_to_fit = true) {
+ if (ARROW_PREDICT_FALSE(new_capacity < 0)) {
+ return Status::Invalid("Resize: negative capacity");
+ }
+ if (ARROW_PREDICT_FALSE(BufferBuilderCapacityTooLarge(new_capacity))) {
+ return Status::CapacityError("Resize: capacity overflow");
+ }
Review Comment:
Why not put these checks in `AllocateResizableBuffer` and
`PoolBuffer::Resize` instead? That would benefit more use cases.
##########
cpp/src/arrow/buffer_builder.h:
##########
@@ -91,11 +102,25 @@ class ARROW_EXPORT BufferBuilder {
/// \param[in] additional_bytes number of additional bytes to make space for
/// \return Status
Status Reserve(const int64_t additional_bytes) {
- auto min_capacity = size_ + additional_bytes;
+ if (ARROW_PREDICT_FALSE(additional_bytes < 0)) {
Review Comment:
That's a lot of similar checks that we're adding in many methods. Can we
find a way of factoring these out, and hopefully avoid forgetting any call
sites where such checks must be done?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
