[arrow] branch main updated: GH-37851: [C++] IPC: ArrayLoader style enhancement (#37872)

[bkietz]

This is an automated email from the ASF dual-hosted git repository.

bkietz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new 816eac4579 GH-37851: [C++] IPC: ArrayLoader style enhancement (#37872)
816eac4579 is described below

commit 816eac4579775b45771fd0f462b73f5a68a87d4b
Author: mwish <maplewish...@gmail.com>
AuthorDate: Sat Sep 30 01:11:33 2023 +0800

    GH-37851: [C++] IPC: ArrayLoader style enhancement (#37872)
    
    
    
    ### Rationale for this change
    
    Enhance the style of `arrow::ipc::ArrayLoader`'s `SkipField`
    
    ### What changes are included in this PR?
    
    Set `out_` to `nullptr` when `SkipField` is called.
    
    ### Are these changes tested?
    
    No
    
    ### Are there any user-facing changes?
    
    No
    
    * Closes: #37851
    
    Authored-by: mwish <maplewish...@gmail.com>
    Signed-off-by: Benjamin Kietzman <bengil...@gmail.com>
---
 cpp/src/arrow/ipc/reader.cc | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/cpp/src/arrow/ipc/reader.cc b/cpp/src/arrow/ipc/reader.cc
index 0def0e036e..6e801e1f8a 100644
--- a/cpp/src/arrow/ipc/reader.cc
+++ b/cpp/src/arrow/ipc/reader.cc
@@ -205,7 +205,10 @@ class ArrayLoader {
     }
   }
 
-  Status LoadType(const DataType& type) { return VisitTypeInline(type, this); }
+  Status LoadType(const DataType& type) {
+    DCHECK_NE(out_, nullptr);
+    return VisitTypeInline(type, this);
+  }
 
   Status Load(const Field* field, ArrayData* out) {
     if (max_recursion_depth_ <= 0) {
@@ -223,6 +226,9 @@ class ArrayLoader {
     skip_io_ = true;
     Status status = Load(field, &dummy);
     skip_io_ = false;
+    // GH-37851: Reset state. Load will set `out_` to `&dummy`, which would
+    // be a dangling pointer.
+    out_ = nullptr;
     return status;
   }
 
@@ -258,6 +264,7 @@ class ArrayLoader {
   }
 
   Status LoadCommon(Type::type type_id) {
+    DCHECK_NE(out_, nullptr);
     // This only contains the length and null count, which we need to figure
     // out what to do with the buffers. For example, if null_count == 0, then
     // we can skip that buffer without reading from shared memory
@@ -276,6 +283,7 @@ class ArrayLoader {
 
   template <typename TYPE>
   Status LoadPrimitive(Type::type type_id) {
+    DCHECK_NE(out_, nullptr);
     out_->buffers.resize(2);
 
     RETURN_NOT_OK(LoadCommon(type_id));
@@ -290,6 +298,7 @@ class ArrayLoader {
 
   template <typename TYPE>
   Status LoadBinary(Type::type type_id) {
+    DCHECK_NE(out_, nullptr);
     out_->buffers.resize(3);
 
     RETURN_NOT_OK(LoadCommon(type_id));
@@ -299,6 +308,7 @@ class ArrayLoader {
 
   template <typename TYPE>
   Status LoadList(const TYPE& type) {
+    DCHECK_NE(out_, nullptr);
     out_->buffers.resize(2);
 
     RETURN_NOT_OK(LoadCommon(type.id()));
@@ -313,6 +323,7 @@ class ArrayLoader {
   }
 
   Status LoadChildren(const std::vector<std::shared_ptr<Field>>& child_fields) 
{
+    DCHECK_NE(out_, nullptr);
     ArrayData* parent = out_;
 
     parent->child_data.resize(child_fields.size());
@@ -2010,7 +2021,7 @@ class StreamDecoder::StreamDecoderImpl : public 
StreamDecoderInternal {
 };
 
 StreamDecoder::StreamDecoder(std::shared_ptr<Listener> listener, 
IpcReadOptions options) {
-  impl_.reset(new StreamDecoderImpl(std::move(listener), options));
+  impl_ = std::make_unique<StreamDecoderImpl>(std::move(listener), options);
 }
 
 StreamDecoder::~StreamDecoder() {}