Jefffrey commented on code in PR #5417:
URL: https://github.com/apache/arrow-rs/pull/5417#discussion_r1497376341
##########
arrow-buffer/src/buffer/mutable.rs:
##########
@@ -67,10 +67,16 @@ impl MutableBuffer {
}
/// Allocate a new [MutableBuffer] with initial capacity to be at least
`capacity`.
+ ///
+ /// # Panics
+ ///
+ /// If `capacity`, when rounded up to a multiple of 64, is larger than
`isize::MAX`
+ /// then this function will panic. Avoid allocating capacities above this
limit.
#[inline]
pub fn with_capacity(capacity: usize) -> Self {
let capacity = bit_util::round_upto_multiple_of_64(capacity);
- let layout = Layout::from_size_align(capacity, ALIGNMENT).unwrap();
+ let layout = Layout::from_size_align(capacity, ALIGNMENT)
+ .expect("failed to create layout for MutableBuffer");
Review Comment:
Panic should come here. Add some documentation around this, and switch to
expect for a slightly better message
##########
arrow-buffer/src/util/bit_util.rs:
##########
@@ -117,6 +117,8 @@ mod tests {
assert_eq!(64, round_upto_multiple_of_64(64));
assert_eq!(128, round_upto_multiple_of_64(65));
assert_eq!(192, round_upto_multiple_of_64(129));
+ // TODO: investigate why this is failing
+ // assert_eq!(usize::MAX, round_upto_multiple_of_64(usize::MAX));
Review Comment:
Found this edge case, don't think it's relevant to the original issue but
might be interesting to check out (since looks like function will round down if
`num` is large enough)
--
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]
