tadeja commented on code in PR #49883:
URL: https://github.com/apache/arrow/pull/49883#discussion_r3545049244
##########
cpp/src/arrow/buffer_builder.h:
##########
@@ -91,11 +92,19 @@ 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)) {
+ return Status::Invalid("Reserve: negative additional_bytes");
+ }
+ int64_t min_capacity;
+ if (ARROW_PREDICT_FALSE(
+ internal::AddWithOverflow(size_, additional_bytes,
&min_capacity))) {
+ return Status::CapacityError("Reserve: capacity overflow");
+ }
if (min_capacity <= capacity_) {
return Status::OK();
}
- return Resize(GrowByFactor(capacity_, min_capacity), false);
+ int64_t requested_capacity = GrowByFactor(capacity_, min_capacity);
+ return Resize(requested_capacity, false);
Review Comment:
```suggestion
return Resize(GrowByFactor(capacity_, min_capacity), false);
```
Small change - this can now go back to one line.
##########
cpp/src/arrow/buffer_builder.h:
##########
Review Comment:
```suggestion
if (ARROW_PREDICT_FALSE(new_capacity < 0)) {
return Status::Invalid("Resize: negative capacity: ", new_capacity);
}
const int64_t old_byte_capacity = bytes_builder_.capacity();
```
Looks like this builder is now without a negative check?
Perhaps add also `TypedBufferBuilder<bool>` test for negative inputs so this
stays covered...
##########
cpp/src/arrow/buffer_test.cc:
##########
@@ -862,6 +875,17 @@ TYPED_TEST(TypedTestBufferBuilder, AppendCopies) {
}
}
+TYPED_TEST(TypedTestBufferBuilder, NegativeAndOverflowAppend) {
+ TypedBufferBuilder<TypeParam> builder;
+
+ ASSERT_RAISES(Invalid, builder.Append(-1, static_cast<TypeParam>(0)));
+
+ const int64_t max_num_elements =
+ std::numeric_limits<int64_t>::max() / sizeof(TypeParam) + 1;
Review Comment:
```suggestion
std::numeric_limits<int64_t>::max() /
static_cast<int64_t>(sizeof(TypeParam)) + 1;
```
--
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]
