scovich commented on code in PR #9816:
URL: https://github.com/apache/arrow-rs/pull/9816#discussion_r3148362780
##########
arrow-data/src/data.rs:
##########
@@ -1554,15 +1578,15 @@ impl ArrayData {
where
T: ArrowNativeType + TryInto<i64> + num_traits::Num +
std::fmt::Display,
{
- let required_len = self.len + self.offset;
+ let required_len = checked_len_plus_offset(&self.data_type, self.len,
self.offset)?;
let buffer = &self.buffers[0];
// This should have been checked as part of `validate()` prior
// to calling `validate_full()` but double check to be sure
assert!(buffer.len() / mem::size_of::<T>() >= required_len);
Review Comment:
aside: Is this a potential panic we should think about turning to an error
instead?
##########
arrow-data/src/data.rs:
##########
@@ -324,7 +336,8 @@ impl ArrayData {
// because we use this buffer to calculate `null_count`
// in `Self::new_unchecked`.
if let Some(null_bit_buffer) = null_bit_buffer.as_ref() {
- let needed_len = bit_util::ceil(len + offset, 8);
+ let len_plus_offset = checked_len_plus_offset(&data_type, len,
offset)?;
+ let needed_len = bit_util::ceil(len_plus_offset, 8);
Review Comment:
Just double checking -- this `bit_util::ceil` cannot overflow because it
always performs a division whose output is smaller than the input (so adding
one cannot overflow). Or, when dividing by 1, the result is returned unchanged
(no need to add one)?
--
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]
