zeroshade commented on code in PR #459:
URL: https://github.com/apache/arrow-nanoarrow/pull/459#discussion_r1594514284


##########
src/nanoarrow/buffer_inline.h:
##########
@@ -468,32 +479,36 @@ static inline void ArrowBitmapMove(struct ArrowBitmap* 
src, struct ArrowBitmap*
 static inline ArrowErrorCode ArrowBitmapReserve(struct ArrowBitmap* bitmap,
                                                 int64_t additional_size_bits) {
   int64_t min_capacity_bits = bitmap->size_bits + additional_size_bits;
-  if (min_capacity_bits <= (bitmap->buffer.capacity_bytes * 8)) {
+  int64_t min_capacity_bytes = _ArrowBytesForBits(min_capacity_bits);
+
+  if (min_capacity_bytes <= bitmap->buffer.capacity_bytes) {
     return NANOARROW_OK;
   }
 
-  NANOARROW_RETURN_NOT_OK(
-      ArrowBufferReserve(&bitmap->buffer, 
_ArrowBytesForBits(additional_size_bits)));
+  int64_t additional_capacity_bytes = min_capacity_bytes - 
bitmap->buffer.size_bytes;
+  NANOARROW_RETURN_NOT_OK(ArrowBufferReserve(&bitmap->buffer, 
additional_capacity_bytes));
+
+  // Zero out the last byte for deterministic output for the common case
+  // of reserving a known remaining size.
+  if (bitmap->buffer.capacity_bytes > 0) {

Review Comment:
   should probably add a test for that exact case, where we have an existing 
bitmap and then we reserve an additional N bits but not enough to force any 
additional bytes, and ensure we don't end up zero-ing out valid data



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to