bkietz commented on a change in pull request #7733:
URL: https://github.com/apache/arrow/pull/7733#discussion_r453830282
##########
File path: cpp/src/arrow/buffer.cc
##########
@@ -43,6 +44,56 @@ Result<std::shared_ptr<Buffer>> Buffer::CopySlice(const
int64_t start,
return std::move(new_buffer);
}
+namespace {
+
+Status CheckBufferSlice(const Buffer& buffer, int64_t offset, int64_t length) {
Review comment:
Maybe this should just be extracted to `int_util.h`
```c++
Status CheckSlice(int64_t original_length, int64_t slice_offset, int64_t
slice_length) {
//...
}
```
then reused for `ArrayData` and `Buffer`.
##########
File path: cpp/src/arrow/array/concatenate.cc
##########
@@ -151,6 +151,10 @@ static Status PutOffsets(const std::shared_ptr<Buffer>&
src, Offset first_offset
return Status::Invalid("offset overflow while concatenating arrays");
}
+ // Concatenate can be called during IPC reads to append delta dictionaries,
+ // on non-validate input. Be sure to avoid reading out of buffer boundaries.
+ // XXX
Review comment:
```suggestion
// Concatenate can be called during IPC reads to append delta dictionaries,
// on input which has not been validated. Be sure to avoid reading out of
// buffer boundaries.
// XXX
```
##########
File path: cpp/src/arrow/array/data.cc
##########
@@ -105,6 +106,22 @@ std::shared_ptr<ArrayData> ArrayData::Slice(int64_t off,
int64_t len) const {
return copy;
}
+Result<std::shared_ptr<ArrayData>> ArrayData::SliceSafe(int64_t off, int64_t
len) const {
+ if (off < 0) {
+ return Status::Invalid("Negative array slice offset");
+ }
+ if (len < 0) {
+ return Status::Invalid("Negative array slice length");
+ }
+ if (internal::HasAdditionOverflow(off, len)) {
+ return Status::Invalid("Array slice would overflow");
+ }
+ if (off + len > length) {
+ return Status::Invalid("Array slice would exceed array length");
+ }
Review comment:
This differs from the behavior of `Slice` which clamps `len` to the
available length (so `[1, 3, 2].Slice(2, 2) == [2]`). Is that intentional? If
so, please add a comment clarifying the difference
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
