jorgecarleitao commented on a change in pull request #8630:
URL: https://github.com/apache/arrow/pull/8630#discussion_r526252139



##########
File path: rust/arrow/src/buffer.rs
##########
@@ -888,6 +888,15 @@ impl MutableBuffer {
         }
         self.len += bytes.len();
     }
+
+    /// Extends the buffer by `len` with all bytes equal to `0u8`, 
incrementing its capacity if needed.
+    pub fn extend(&mut self, len: usize) {
+        let remaining_capacity = self.capacity - self.len;
+        if len > remaining_capacity {
+            self.reserve(self.len + len);

Review comment:
       I am sorry, I do not follow: my understanding is that we need to 
`reserve` so that `self.len < self.capacity` is always true.
   
   If we have a buffer with capacity 64 and `len` 54, and we extend it by 8, we 
want to ensure that its new capacity is at least `54+8=62`, no?
   
   your formula yields `54 + (8 - (64 - 54)) = 54 + (8 - 10) = 52` (panics with 
an underflow, I think).
   
   Another way of writing this code would be:
   ```
   self.len += len;
   if self.len > self.capacity {
       self.reserve(self.len);
   }
   ```




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to