Use smart pointers up to and including _deliver(). Still dumb pointers in the functor and above.

native/src/impl/file.cpp

@@ -154,9 +154,14 @@ FileADT::_validate_readv(const ReadList& requests, std::int64_t eof, OpenMode mo
  *
  * If the functor states that it needs to retain a pointer then make
  * sure it gets a smart pointer that is aliased to the entire buffer.
+ *
+ * Transition aid, TO BE REMOVED.
+ * In some places the caller of _deliver() only has an unsafe pointer.
+ * Typically because it got that pointer from another delivery, and
+ * the delivery functor itself still just provides a dumb pointer.
  */
 void
-FileADT::_deliver(
+FileADT::_deliver_old(
      const ReadRequest::delivery_t& fn,
      ReadRequest::data_t data,
      std::int64_t offset,
@@ -176,6 +181,17 @@ FileADT::_deliver(
   }
 }
 
+void
+FileADT::_deliver(
+     const ReadRequest::delivery_t& fn,
+     const std::shared_ptr<const void>&  data,
+     std::int64_t offset,
+     std::int64_t size,
+     bool transient)
+{
+  _deliver_old(fn, data.get(), offset, size, transient);
+}
+
 std::shared_ptr<FileADT>
 FileADT::factory(const std::string& filename, OpenMode mode, const IOContext *iocontext)
 {

native/src/impl/file.h

@@ -234,7 +234,8 @@ protected:
   static void _validate_write(const void *data, std::int64_t offset, std::int64_t size, OpenMode mode);
   static void _validate_readv(const ReadList& requests, std::int64_t eof, OpenMode mode);
 public: // Actually internal. Used by ConsolidateRequests.
-  static void _deliver(const ReadRequest::delivery_t& fn, ReadRequest::data_t data, std::int64_t offset, std::int64_t size, bool transient);
+  static void _deliver_old(const ReadRequest::delivery_t& fn, ReadRequest::data_t data, std::int64_t offset, std::int64_t size, bool transient);
+  static void _deliver(const ReadRequest::delivery_t& fn, const std::shared_ptr<const void>& data, std::int64_t offset, std::int64_t size, bool transient);
 
 public:
   static std::shared_ptr<FileADT> factory(const std::string& filename, OpenMode mode, const OpenZGY::IOContext *iocontext);

native/src/impl/file_consolidate.cpp

@@ -33,7 +33,7 @@ namespace InternalZGY {
  *
  * COMPLEX TYPES:
  *
- *    class ReadRequest {offset, size, delivery_functor(buffer*, size)}
+ *    class ReadRequest {offset, size, delivery_functor(buffer, size)}
  *    class RawRequest  {seg_number, offset_in_seg, size_in_seg, outpos}
  *
  * FUNCTION CALL HIERARCHY:
@@ -363,7 +363,7 @@ ConsolidateRequests::_consolidated_delivery(
                std::int64_t end = std::min(rr.offset + rr.size - begin, size);
                std::int64_t beg = std::min(rr.offset - begin, end);
                // Caller will check the transient_ok flag. we won't.
-               FileADT::_deliver(rr.delivery, data, beg, end - beg, false);
+               FileADT::_deliver_old(rr.delivery, data, beg, end - beg, false);
              }
            }
          };

native/src/impl/file_local.cpp

@@ -258,7 +258,7 @@ LocalFileLinux::xx_readv(const ReadList& requests, bool parallel_ok, bool immuta
     for (const ReadRequest& r : requests) {
       std::shared_ptr<char> data(new char[r.size]);
       this->LocalFileLinux::xx_read(data.get(), r.offset, r.size, usagehint);
-      _deliver(r.delivery, data.get(), 0, r.size, transient_ok);
+      _deliver(r.delivery, data, 0, r.size, transient_ok);
     }
   }
   else {
@@ -298,7 +298,7 @@ LocalFileLinux::xx_readv(const ReadList& requests, bool parallel_ok, bool immuta
         const ReadRequest& r = requests[ii];
         guard.run([&](){
           this->LocalFileLinux::xx_read(data.get(), r.offset, r.size, usagehint);
-          _deliver(r.delivery, data.get(), 0, r.size, transient_ok);
+          _deliver(r.delivery, data, 0, r.size, transient_ok);
         });
       }
     }

native/src/impl/file_parallelizer.cpp

@@ -132,7 +132,7 @@ FileParallelizer::xx_readv(
   for (std::int64_t ii = 0; ii < requestcount; ++ii) {
     guard.run([&](){
       std::cerr << "0123456789"[omp_get_thread_num() % 10];
-      _deliver(requests[ii].delivery, buffers[ii].get(), 0, requests[ii].size, transient_ok);
+      _deliver(requests[ii].delivery, buffers[ii], 0, requests[ii].size, transient_ok);
     });
   }
   guard.finished();

native/src/impl/file_sd.cpp

@@ -1103,8 +1103,8 @@ SeismicStoreFile::xx_make_instance(const std::string& filename, OpenMode mode, c
 
     // Improve multi-threading of decompress and copy-out.
     auto context = dynamic_cast<const SeismicStoreIOContext*>(iocontext);
-  if (context && context->_cputhreads > 1)
-    file = FileParallelizer::inject(file, context->_cputhreads);
+    if (context && context->_cputhreads > 1)
+      file = FileParallelizer::inject(file, context->_cputhreads);
 
     return file;
   }
@@ -1155,7 +1155,7 @@ SeismicStoreFile::xx_readv(const ReadList& requests, bool parallel_ok, bool immu
   for (const ReadRequest& r : requests) {
     std::shared_ptr<char> data(new char[r.size]);
     this->SeismicStoreFile::xx_read(data.get(), r.offset, r.size, usagehint);
-    _deliver(r.delivery, data.get(), 0, r.size, transient_ok);
+    _deliver(r.delivery, data, 0, r.size, transient_ok);
   }
   return;
 #endif
@@ -1296,7 +1296,7 @@ SeismicStoreFile::xx_readv(const ReadList& requests, bool parallel_ok, bool immu
     // TODO-Worry: If this_size != rr.size, can this ever happen?
     // If yes then we might have lost track of where in the buffer
     // we should copy out from. This wory also applies to the Python code.
-    _deliver(rr.delivery, data.get(), pos, this_size, transient_ok);
+    _deliver(rr.delivery, data, pos, this_size, transient_ok);
     pos += this_size;
   }
 }
@@ -1809,7 +1809,7 @@ SeismicStoreFileDelayedWrite::xx_readv(const ReadList& requests, bool parallel_o
     for (const ReadRequest& r : requests) {
       std::shared_ptr<char> data(new char[r.size]);
       this->SeismicStoreFileDelayedWrite::xx_read(data.get(), r.offset, r.size, usagehint);
-      _deliver(r.delivery, data.get(), 0, r.size, transient_ok);
+      _deliver(r.delivery, data, 0, r.size, transient_ok);
     }
   }
 }

native/src/impl/file_windows.cpp

@@ -237,7 +237,7 @@ LocalFileWindows::xx_readv(const ReadList& requests, bool parallel_ok, bool immu
   for (const ReadRequest& r : requests) {
     std::unique_ptr<char> data(new char[r.size]);
     this->LocalFileWindows::xx_read(data.get(), r.offset, r.size, usagehint);
-    _deliver(r.delivery, data.get(), 0, r.size, transient_ok);
+    _deliver(r.delivery, data, 0, r.size, transient_ok);
   }
 }
 

native/src/test/test_consolidate.cpp

@@ -169,13 +169,13 @@ run_test(const ReadList& list_in,
 
   // Pretend we read this data file a file. The raw data contains the
   // offset from the start of the buffer so we can check what was sent.
-  std::vector<int> data(filesize/sizeof(int), 0);
-  for (std::size_t ii=0; ii<data.size(); ++ii)
-    data[ii] = sizeof(int)*static_cast<int>(ii);
+  std::shared_ptr<int> data(new int[filesize/sizeof(int)]);
+  for (std::size_t ii=0; ii<filesize/sizeof(int); ++ii)
+    data.get()[ii] = sizeof(int)*static_cast<int>(ii);
 
   // Deliver my data according to the consolidated list.
   for (const ReadRequest& rr : result)
-    FileADT::_deliver(rr.delivery, data.data(), rr.offset, rr.size, false);
+    FileADT::_deliver(rr.delivery, data, rr.offset, rr.size, false);
 
   // Check that all the data that was originally requested
   // was delivered to the correct place in a timely manner.