[ARROW-16238] [C++] Fix nullptr deference in ipc/reader.cc

• Type: Bug
• Status: Resolved
• Priority: Critical
• Resolution: Fixed
• Fix Version/s: 8.0.0
• Component/s: C++
• Labels: pull-request-available
• External issue URL: https://github.com/apache/arrow/issues/31634
• Assignee: David Li
• Reporter: David Li
• Created: 19/Apr/22 16:11
• Updated: 11/Jan/23 11:42
• Resolved: 23/Apr/22 08:13

Description

MinGW GCC catches this

[20/278] Building CXX object src/arrow/CMakeFiles/arrow_shared.dir/Unity/unity_24_cxx.cxx.obj
In file included from C:/msys64/home/User/arrow/build/cpp/src/arrow/CMakeFiles/arrow_shared.dir/Unity/unity_24_cxx.cxx:3:
C:/msys64/home/User/arrow/cpp/src/arrow/ipc/reader.cc: In member function 'virtual arrow::Result<std::function<arrow::Future<std::shared_ptr<arrow::RecordBatch> >()> > arrow::ipc::RecordBatchFileReaderImpl::GetRecordBatchGenerator(bool, const arrow::io::IOContext&, arrow::io::CacheOptions, arrow::internal::Executor*)':
C:/msys64/home/User/arrow/cpp/src/arrow/ipc/reader.cc:1303:34: warning: 'this' pointer is null [-Wnonnull]
 1303 |       return cached_source->Cache({{0, footer_offset_}});
      |              ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
In file included from C:/msys64/home/User/arrow/cpp/src/arrow/ipc/reader.h:28,
                 from C:/msys64/home/User/arrow/cpp/src/arrow/ipc/reader.cc:18,
                 from C:/msys64/home/User/arrow/build/cpp/src/arrow/CMakeFiles/arrow_shared.dir/Unity/unity_24_cxx.cxx:3:
C:/msys64/home/User/arrow/cpp/src/arrow/io/caching.h:124:10: note: in a call to non-static member function 'arrow::Status arrow::io::internal::ReadRangeCache::Cache(std::vector<arrow::io::ReadRange>)'
  124 |   Status Cache(std::vector<ReadRange> ranges);
      |          ^~~~~ 

This is pretty clearly wrong:

    std::shared_ptr<io::internal::ReadRangeCache> cached_source;
    if (coalesce && file_->supports_zero_copy()) {
      if (!owned_file_) return Status::Invalid("Cannot coalesce without an owned file");
      // Since the user is asking for all fields then we can cache the entire
      // file (up to the footer)
      return cached_source->Cache({{0, footer_offset_}});
    }
    return WholeIpcFileRecordBatchGenerator(std::move(state), std::move(cached_source),
                                            io_context, executor);

It seems ARROW-14577 removed one too many lines

Issue Links

• links to: GitHub Pull Request #12937