rustyconover commented on code in PR #955:
URL: https://github.com/apache/arrow-nanoarrow/pull/955#discussion_r4254490186


##########
src/nanoarrow/ipc/decoder.c:
##########
@@ -337,16 +337,74 @@ static ArrowErrorCode ArrowIpcDictionaryReplace(struct 
ArrowIpcDictionary* dicti
   return NANOARROW_OK;
 }
 
+// Point dictionaries of dst (and its children) at those of src, which reflect
+// the most recent value of any nested dictionaries.
+static ArrowErrorCode ArrowIpcArraySetDictionaries(struct ArrowArray* dst,
+                                                   const struct ArrowArray* 
src) {
+  if (src->dictionary != NULL) {
+    NANOARROW_DCHECK(dst->dictionary != NULL);
+    if (dst->dictionary->release != NULL) {
+      ArrowArrayRelease(dst->dictionary);
+    }
+    NANOARROW_RETURN_NOT_OK(ArrowArrayCloneShared(src->dictionary, 
dst->dictionary));
+  }
+
+  for (int64_t i = 0; i < src->n_children; i++) {
+    NANOARROW_RETURN_NOT_OK(
+        ArrowIpcArraySetDictionaries(dst->children[i], src->children[i]));
+  }
+
+  return NANOARROW_OK;
+}
+
+// Append the values of current and then value into combined, which must have 
been
+// initialized from array_view. array_view is used as scratch space to view 
current
+// and value.
+static ArrowErrorCode ArrowIpcDictionaryConcatenate(struct ArrowArray* 
combined,
+                                                    const struct ArrowArray* 
current,
+                                                    const struct ArrowArray* 
value,
+                                                    struct ArrowArrayView* 
array_view,
+                                                    struct ArrowError* error) {
+  NANOARROW_RETURN_NOT_OK_WITH_ERROR(ArrowArrayStartAppending(combined), 
error);
+  NANOARROW_RETURN_NOT_OK_WITH_ERROR(
+      ArrowArrayReserve(combined, current->length + value->length), error);

Review Comment:
   Fixed in 544b501c64279f3af336c6528f099b7c0958efbb. 
`ArrowIpcDictionaryConcatenate()` now checks `current->length > INT64_MAX - 
value->length` before adding the lengths, returning `EOVERFLOW` with both 
lengths in the error message.
   
   The regression test uses null dictionaries to cover `INT64_MAX + 1` and `1 + 
INT64_MAX` through both dictionary decoding APIs. It also checks that rejection 
preserves the current dictionary and that subsequent replacement and delta 
decoding still work. With the guard removed, the test reproduces the signed 
overflow under UBSan; with the fix, it passes.
   
   Also merged the latest `main` (ea215b0) as requested, triggering fresh CI.
   
   Built and tested on aarch64 Linux EC2, Debug with ZSTD/LZ4:
   
   - Arrow C++ 24.0.0: 857 tests passed, 13 skipped, no failures.
   - Without Arrow C++, with UBSan: all 387 tests passed (serial CTest).
   - Decoder suite under Valgrind: 301 passed, 10 skipped; zero errors and no 
leaks.
   
   Formatting checked with clang-format 16.0.6.



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