OmBiradar opened a new issue, #50311:
URL: https://github.com/apache/arrow/issues/50311
### Describe the bug, including details regarding any error messages,
version, and platform.
I was going through `KeyValueMetadata` and found some error handling which I
feel can be made better.
Like for `KeyValueMetadata`
```cpp
Status KeyValueMetadata::Delete(int64_t index) {
keys_.erase(keys_.begin() + index);
values_.erase(values_.begin() + index);
return Status::OK();
}
```
Here if the index is out of bounds, i.e. `index >= keys_.size()` or `index <
0`, then this will throw a seg fault or may corrupt memory.
Would a fix like the below would be good? If so I will open a PR for it.
I also used `ARROW_PREDICT_FALSE` to minimise the overhead.
```cpp
Status KeyValueMetadata::Delete(int64_t index) {
if (ARROW_PREDICT_FALSE(index < 0 || index >=
static_cast<int64_t>(values_.size()))) {
return arrow::Status::IndexError("Index out of bounds. Expected 0 to ",
std::to_string(keys_.size() - 1), ", but got ", std::to_string(index));
}
keys_.erase(keys_.begin() + index);
values_.erase(values_.begin() + index);
return Status::OK();
}
```
Also in C++20, the `static_cast` above can be changed with
`std::cmp_greater_equal(index, values_.size())` but I see that Arrow supports a
minimum of C++17
### Component(s)
C++
--
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]
