Dandandan commented on code in PR #628:
URL: 
https://github.com/apache/arrow-rs-object-store/pull/628#discussion_r2742320131


##########
src/local.rs:
##########
@@ -988,26 +988,79 @@ pub(crate) fn read_range(
 
     // Don't read past end of file
     let to_read = range.end.min(file_len) - range.start;
+    let mut buf = Vec::with_capacity(to_read as usize);
 
-    file.seek(SeekFrom::Start(range.start)).map_err(|source| {
-        let path = path.into();
-        Error::Seek { source, path }
-    })?;
+    #[cfg(any(target_family = "unix", target_family = "windows"))]
+    {
+        #[cfg(target_family = "unix")]
+        use std::os::unix::fs::FileExt;
+        #[cfg(target_family = "windows")]
+        use std::os::windows::fs::FileExt;
+
+        // Safety:
+        // Setting the buffer's length to its capacity is safe as it remains 
within its allocated memory.
+        // In cases where `read_exact_at` errors, the contents of the buffer 
are undefined,
+        // but we discard it without using it.
+        unsafe {
+            buf.set_len(to_read as usize);
+        }

Review Comment:
   Or a suggestion by Gemini:
   
   ```
   use std::os::unix::io::AsRawFd;
   
   let fd = file.as_raw_fd();
   let mut buf = Vec::with_capacity(to_read);
   
   let bytes_read = unsafe {
       // libc::pread takes a raw pointer (*mut c_void), avoiding the reference 
creation UB
       libc::pread(
           fd, 
           buf.as_mut_ptr() as *mut _, 
           to_read, 
           offset as i64
       )
   };
   
   if bytes_read >= 0 {
       unsafe { buf.set_len(bytes_read as usize) };
   } else {
       // Handle error
   }
   ```



##########
src/local.rs:
##########
@@ -988,26 +988,79 @@ pub(crate) fn read_range(
 
     // Don't read past end of file
     let to_read = range.end.min(file_len) - range.start;
+    let mut buf = Vec::with_capacity(to_read as usize);
 
-    file.seek(SeekFrom::Start(range.start)).map_err(|source| {
-        let path = path.into();
-        Error::Seek { source, path }
-    })?;
+    #[cfg(any(target_family = "unix", target_family = "windows"))]
+    {
+        #[cfg(target_family = "unix")]
+        use std::os::unix::fs::FileExt;
+        #[cfg(target_family = "windows")]
+        use std::os::windows::fs::FileExt;
+
+        // Safety:
+        // Setting the buffer's length to its capacity is safe as it remains 
within its allocated memory.
+        // In cases where `read_exact_at` errors, the contents of the buffer 
are undefined,
+        // but we discard it without using it.
+        unsafe {
+            buf.set_len(to_read as usize);
+        }

Review Comment:
   Or a suggestion by Gemini (raw file IO):
   
   ```
   use std::os::unix::io::AsRawFd;
   
   let fd = file.as_raw_fd();
   let mut buf = Vec::with_capacity(to_read);
   
   let bytes_read = unsafe {
       // libc::pread takes a raw pointer (*mut c_void), avoiding the reference 
creation UB
       libc::pread(
           fd, 
           buf.as_mut_ptr() as *mut _, 
           to_read, 
           offset as i64
       )
   };
   
   if bytes_read >= 0 {
       unsafe { buf.set_len(bytes_read as usize) };
   } else {
       // Handle error
   }
   ```



-- 
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]

Reply via email to