crepererum commented on code in PR #8452:
URL: https://github.com/apache/arrow-rs/pull/8452#discussion_r2382260382
##########
arrow-buffer/src/buffer/immutable.rs:
##########
@@ -181,12 +181,14 @@ impl Buffer {
len: usize,
deallocation: Deallocation,
) -> Self {
- let bytes = Bytes::new(ptr, len, deallocation);
- let ptr = bytes.as_ptr();
- Buffer {
- ptr,
- data: Arc::new(bytes),
- length: len,
+ unsafe {
Review Comment:
unsafe block should ideally only wrap the actual unsafe call/op
##########
arrow-buffer/src/buffer/mutable.rs:
##########
@@ -458,11 +458,13 @@ impl MutableBuffer {
/// Caller must ensure that the capacity()-len()>=`size_of<T>`()
#[inline]
pub unsafe fn push_unchecked<T: ToByteSlice>(&mut self, item: T) {
- let additional = std::mem::size_of::<T>();
- let src = item.to_byte_slice().as_ptr();
- let dst = self.data.as_ptr().add(self.len);
- std::ptr::copy_nonoverlapping(src, dst, additional);
- self.len += additional;
+ unsafe {
Review Comment:
same
##########
arrow-buffer/src/buffer/mutable.rs:
##########
@@ -629,27 +631,29 @@ impl MutableBuffer {
pub unsafe fn from_trusted_len_iter<T: ArrowNativeType, I: Iterator<Item =
T>>(
iterator: I,
) -> Self {
- let item_size = std::mem::size_of::<T>();
- let (_, upper) = iterator.size_hint();
- let upper = upper.expect("from_trusted_len_iter requires an upper
limit");
- let len = upper * item_size;
-
- let mut buffer = MutableBuffer::new(len);
-
- let mut dst = buffer.data.as_ptr();
- for item in iterator {
- // note how there is no reserve here (compared with
`extend_from_iter`)
- let src = item.to_byte_slice().as_ptr();
- std::ptr::copy_nonoverlapping(src, dst, item_size);
- dst = dst.add(item_size);
+ unsafe {
Review Comment:
same, here's it's esp. bad since I'm not even sure where the unsafe part is
##########
arrow-buffer/src/util/bit_mask.rs:
##########
@@ -130,21 +130,23 @@ unsafe fn set_upto_64bits(
/// The caller must ensure `data` has `offset..(offset + 8)` range, and `count
<= 8`.
#[inline]
unsafe fn read_bytes_to_u64(data: &[u8], offset: usize, count: usize) -> u64 {
- debug_assert!(count <= 8);
- let mut tmp: u64 = 0;
- let src = data.as_ptr().add(offset);
unsafe {
+ debug_assert!(count <= 8);
+ let mut tmp: u64 = 0;
+ let src = data.as_ptr().add(offset);
std::ptr::copy_nonoverlapping(src, &mut tmp as *mut _ as *mut u8,
count);
+ tmp
}
- tmp
}
/// # Safety
/// The caller must ensure `data` has `offset..(offset + 8)` range
#[inline]
unsafe fn write_u64_bytes(data: &mut [u8], offset: usize, chunk: u64) {
- let ptr = data.as_mut_ptr().add(offset) as *mut u64;
- ptr.write_unaligned(chunk);
+ unsafe {
Review Comment:
same
##########
arrow-buffer/src/util/bit_mask.rs:
##########
@@ -130,21 +130,23 @@ unsafe fn set_upto_64bits(
/// The caller must ensure `data` has `offset..(offset + 8)` range, and `count
<= 8`.
#[inline]
unsafe fn read_bytes_to_u64(data: &[u8], offset: usize, count: usize) -> u64 {
- debug_assert!(count <= 8);
- let mut tmp: u64 = 0;
- let src = data.as_ptr().add(offset);
unsafe {
Review Comment:
same
##########
arrow-buffer/src/buffer/mutable.rs:
##########
@@ -691,33 +695,37 @@ impl MutableBuffer {
>(
iterator: I,
) -> Result<Self, E> {
- let item_size = std::mem::size_of::<T>();
- let (_, upper) = iterator.size_hint();
- let upper = upper.expect("try_from_trusted_len_iter requires an upper
limit");
- let len = upper * item_size;
-
- let mut buffer = MutableBuffer::new(len);
-
- let mut dst = buffer.data.as_ptr();
- for item in iterator {
- let item = item?;
- // note how there is no reserve here (compared with
`extend_from_iter`)
- let src = item.to_byte_slice().as_ptr();
- std::ptr::copy_nonoverlapping(src, dst, item_size);
- dst = dst.add(item_size);
- }
- // try_from_trusted_len_iter is instantiated a lot, so we extract part
of it into a less
- // generic method to reduce compile time
- unsafe fn finalize_buffer(dst: *mut u8, buffer: &mut MutableBuffer,
len: usize) {
- assert_eq!(
- dst.offset_from(buffer.data.as_ptr()) as usize,
- len,
- "Trusted iterator length was not accurately reported"
- );
- buffer.len = len;
+ unsafe {
Review Comment:
same
##########
arrow-buffer/src/util/bit_mask.rs:
##########
@@ -154,9 +156,11 @@ unsafe fn write_u64_bytes(data: &mut [u8], offset: usize,
chunk: u64) {
/// The caller must ensure `data` has `offset..(offset + 8)` range
#[inline]
unsafe fn or_write_u64_bytes(data: &mut [u8], offset: usize, chunk: u64) {
- let ptr = data.as_mut_ptr().add(offset);
- let chunk = chunk | (*ptr) as u64;
- (ptr as *mut u64).write_unaligned(chunk);
+ unsafe {
Review Comment:
same
--
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]
