cleanups

apache · Sep 13, 2024 · 1e0019d · 1e0019d
1 parent 6d79e7f
commit 1e0019d
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 27 deletions.
diff --git a/src/nanoarrow/common/array.c b/src/nanoarrow/common/array.c
@@ -18,7 +18,6 @@
 #include <errno.h>
 #include <inttypes.h>
 #include <stdarg.h>
-#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -463,16 +462,16 @@ static ArrowErrorCode ArrowArrayFinalizeBuffers(struct ArrowArray* array) {
     NANOARROW_RETURN_NOT_OK(ArrowArrayFinalizeBuffers(array->dictionary));
   }
 
-  const int32_t n_vbuf = private_data->n_variadic_buffers;
-  if (n_vbuf > 0) {
-    // TODO: the hard-coded 2's are tied to binary/string view implementations
-    // but maybe we should make them generic
-    private_data->buffer_data = (const void**)realloc(private_data->buffer_data,
-                                                      sizeof(void*) * (2 + n_vbuf + 1));
+  if (private_data->storage_type == NANOARROW_TYPE_STRING_VIEW ||
+      private_data->storage_type == NANOARROW_TYPE_BINARY_VIEW) {
+    const int32_t nfixed_buf = 2;
+    const int32_t n_vbuf = private_data->n_variadic_buffers;
+    private_data->buffer_data = (const void**)realloc(
+        private_data->buffer_data, sizeof(void*) * (nfixed_buf + n_vbuf + 1));
     for (int32_t i = 0; i < n_vbuf; ++i) {
-      private_data->buffer_data[2 + i] = private_data->variadic_buffers[i].data;
+      private_data->buffer_data[nfixed_buf + i] = private_data->variadic_buffers[i].data;
     }
-    private_data->buffer_data[n_vbuf + 2] = private_data->variadic_buffer_sizes;
+    private_data->buffer_data[nfixed_buf + n_vbuf] = private_data->variadic_buffer_sizes;
     array->buffers = (const void**)(private_data->buffer_data);
   }
 
@@ -483,11 +482,13 @@ static void ArrowArrayFlushInternalPointers(struct ArrowArray* array) {
   struct ArrowArrayPrivateData* private_data =
       (struct ArrowArrayPrivateData*)array->private_data;
 
-  // TODO: this does not seem correct - should probably merge changes from
-  // ArrowArrayFinalizeBuffers into this
-  const int32_t limit =
-      private_data->n_variadic_buffers == 0 ? NANOARROW_MAX_FIXED_BUFFERS : 2;
-  for (int32_t i = 0; i < limit; i++) {
+  const int32_t nfixed_buf =
+      private_data->storage_type == NANOARROW_TYPE_STRING_VIEW ||
+              private_data->storage_type == NANOARROW_TYPE_BINARY_VIEW
+          ? 2
+          : NANOARROW_MAX_FIXED_BUFFERS;
+
+  for (int32_t i = 0; i < nfixed_buf; i++) {
     private_data->buffer_data[i] = ArrowArrayBuffer(array, i)->data;
   }
 
@@ -736,7 +737,11 @@ static int ArrowArrayViewSetArrayInternal(struct ArrowArrayView* array_view,
   array_view->n_variadic_buffers = 0;
 
   int64_t buffers_required = 0;
-  for (int i = 0; i < NANOARROW_MAX_FIXED_BUFFERS; i++) {
+  const int nfixed_buf = array_view->storage_type == NANOARROW_TYPE_STRING_VIEW ||
+                                 array_view->storage_type == NANOARROW_TYPE_BINARY_VIEW
+                             ? 2
+                             : NANOARROW_MAX_FIXED_BUFFERS;
+  for (int i = 0; i < nfixed_buf; i++) {
     if (array_view->layout.buffer_type[i] == NANOARROW_BUFFER_TYPE_NONE) {
       break;
     }
@@ -757,17 +762,14 @@ static int ArrowArrayViewSetArrayInternal(struct ArrowArrayView* array_view,
   if (array_view->storage_type == NANOARROW_TYPE_STRING_VIEW ||
       array_view->storage_type == NANOARROW_TYPE_BINARY_VIEW) {
     const int64_t n_buffers = array->n_buffers;
-    const int32_t n_fixed_buffers = 2;
+    const int32_t nfixed_buf = 2;
 
-    // TODO: bounds checking
-    int32_t n_variadic_buffers = (int32_t)(n_buffers - n_fixed_buffers - 1);
-    // Theoretically if the variadic buffers are not used the above subtraction
-    // could yield a negative number
-    n_variadic_buffers = (n_variadic_buffers < 0) ? 0 : n_variadic_buffers;
-    array_view->n_variadic_buffers = n_variadic_buffers;
+    int32_t nvariadic_buf = (int32_t)(n_buffers - nfixed_buf - 1);
+    nvariadic_buf = (nvariadic_buf < 0) ? 0 : nvariadic_buf;
+    array_view->n_variadic_buffers = nvariadic_buf;
 
-    buffers_required += n_variadic_buffers;
-    for (int i = n_fixed_buffers; i < n_fixed_buffers + n_variadic_buffers; ++i) {
+    buffers_required += nvariadic_buf;
+    for (int i = nfixed_buf; i < nfixed_buf + nvariadic_buf; ++i) {
       // Set buffer pointer
       array_view->buffer_views[i].data.data = array->buffers[i];
 

diff --git a/src/nanoarrow/common/inline_array.h b/src/nanoarrow/common/inline_array.h
@@ -502,7 +502,6 @@ static inline ArrowErrorCode ArrowArrayAppendBytes(struct ArrowArray* array,
       private_data->storage_type == NANOARROW_TYPE_BINARY_VIEW) {
     struct ArrowBuffer* data_buffer = ArrowArrayBuffer(array, 1);
     union ArrowBinaryViewType bvt;
-    // TODO: bounds check
     bvt.inlined.size = (int32_t)value.size_bytes;
 
     if (value.size_bytes <= NANOARROW_BINARY_VIEW_INLINE_SIZE) {
@@ -525,11 +524,9 @@ static inline ArrowErrorCode ArrowArrayAppendBytes(struct ArrowArray* array,
       struct ArrowBuffer* variadic_buf = &private_data->variadic_buffers[n_vbufs - 1];
       memcpy(bvt.ref.data, value.data.as_char, NANOARROW_BINARY_VIEW_PREVIEW_SIZE);
       bvt.ref.buffer_index = n_vbufs - 1;
-      // TODO: bounds check
       bvt.ref.offset = (int32_t)variadic_buf->size_bytes;
       NANOARROW_RETURN_NOT_OK(
           ArrowBufferAppend(variadic_buf, value.data.as_char, value.size_bytes));
-      // TODO: bounds check
       private_data->variadic_buffer_sizes[n_vbufs - 1] =
           (int32_t)variadic_buf->size_bytes;
     }