jhorstmann commented on code in PR #6021:
URL: https://github.com/apache/arrow-rs/pull/6021#discussion_r1669963396


##########
parquet/src/arrow/array_reader/byte_view_array.rs:
##########
@@ -315,26 +315,30 @@ impl ByteViewArrayDecoderPlain {
         let mut read = 0;
         output.views.reserve(to_read);
         while self.offset < self.buf.len() && read != to_read {
-            if self.offset + 4 > self.buf.len() {
-                return Err(ParquetError::EOF("eof decoding byte 
array".into()));
-            }
-            let len_bytes: [u8; 4] = unsafe {
-                buf.get_unchecked(self.offset..self.offset + 4)
-                    .try_into()
-                    .unwrap()
+            let len_bytes = buf.get(self.offset..self.offset + 4);
+            let len = match len_bytes {
+                None => {
+                    return Err(ParquetError::EOF("eof decoding byte 
array".into()));
+                }
+                Some(l) => u32::from_le_bytes(l.try_into().unwrap()),

Review Comment:
   I think allocation sizes in rust are limited to `isize::MAX`, so `offset+4` 
should never overflow.
   
   Personally, I find the previous version with the simple `if` a bit easier to 
read. The `unsafe` can probably be removed since the compiler should be able to 
remove the bounds checks on its own:
   
   ```
   let len = u32::from_le_bytes(buf[offset..offset+4].try_into().unwrap())
   ```



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