From 9f3240c91cf4913baaf874615f58d357299f09e5 Mon Sep 17 00:00:00 2001
From: Kevin Backhouse <kevinbackhouse@github.com>
Date: Fri, 24 Dec 2021 12:32:48 +0000
Subject: [PATCH] Delete TiffEntryBase::isMalloced_ field and use a
 std::shared_ptr instead.

---
 src/tiffcomposite_int.cpp | 25 +++++++------------------
 src/tiffcomposite_int.hpp | 21 ++++++++++++++++++---
 src/tiffvisitor_int.cpp   |  6 ++++--
 3 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/src/tiffcomposite_int.cpp b/src/tiffcomposite_int.cpp
index 943f33bb..66434733 100644
--- a/src/tiffcomposite_int.cpp
+++ b/src/tiffcomposite_int.cpp
@@ -97,7 +97,6 @@ namespace Exiv2 {
           offset_(0),
           size_(0),
           pData_(nullptr),
-          isMalloced_(false),
           idx_(0),
           pValue_(nullptr)
     {
@@ -188,9 +187,6 @@ namespace Exiv2 {
 
     TiffEntryBase::~TiffEntryBase()
     {
-        if (isMalloced_) {
-            delete[] pData_;
-        }
         delete pValue_;
     }
 
@@ -218,14 +214,10 @@ namespace Exiv2 {
           offset_(rhs.offset_),
           size_(rhs.size_),
           pData_(rhs.pData_),
-          isMalloced_(rhs.isMalloced_),
           idx_(rhs.idx_),
-          pValue_(rhs.pValue_ ? rhs.pValue_->clone().release() : nullptr)
+          pValue_(rhs.pValue_ ? rhs.pValue_->clone().release() : nullptr),
+          storage_(rhs.storage_)
     {
-        if (rhs.isMalloced_) {
-            pData_ = new byte[rhs.size_];
-            memcpy(pData_, rhs.pData_, rhs.size_);
-        }
     }
 
     TiffDirectory::TiffDirectory(const TiffDirectory& rhs) : TiffComponent(rhs), hasNext_(rhs.hasNext_), pNext_(nullptr)
@@ -320,18 +312,15 @@ namespace Exiv2 {
         return idx_;
     }
 
-    void TiffEntryBase::setData(DataBuf buf)
+    void TiffEntryBase::setData(const std::shared_ptr<DataBuf>& buf)
     {
-        std::pair<byte*, long> p = buf.release();
-        setData(p.first, p.second);
-        isMalloced_ = true;
+        storage_ = buf;
+        pData_ = buf->data();
+        size_ = buf->size();
     }
 
     void TiffEntryBase::setData(byte* pData, int32_t size)
     {
-        if (isMalloced_) {
-            delete[] pData_;
-        }
         pData_ = pData;
         size_  = size;
         if (pData_ == nullptr)
@@ -344,7 +333,7 @@ namespace Exiv2 {
             return;
         uint32_t newSize = value->size();
         if (newSize > size_) {
-            setData(DataBuf(newSize));
+            setData(std::make_shared<DataBuf>(newSize));
         }
         if (pData_ != nullptr) {
             memset(pData_, 0x0, size_);
diff --git a/src/tiffcomposite_int.hpp b/src/tiffcomposite_int.hpp
index 1db4f520..74b7a6e5 100644
--- a/src/tiffcomposite_int.hpp
+++ b/src/tiffcomposite_int.hpp
@@ -438,8 +438,12 @@ namespace Exiv2 {
         void setOffset(int32_t offset) { offset_ = offset; }
         //! Set pointer and size of the entry's data (not taking ownership of the data).
         void setData(byte* pData, int32_t size);
-        //! Set the entry's data buffer, taking ownership of the data buffer passed in.
-        void setData(DataBuf buf);
+        /*!
+          @brief Set the entry's data buffer. A shared_ptr is used to manage the DataBuf
+                 because TiffEntryBase has a clone method so it is possible (in theory) for
+                 the DataBuf to have multiple owners.
+         */
+        void setData(const std::shared_ptr<DataBuf>& buf);
          /*!
           @brief Update the value. Takes ownership of the pointer passed in.
 
@@ -545,11 +549,22 @@ namespace Exiv2 {
           minimum size.
          */
         uint32_t size_;
+
+        // Notes on the ownership model of pData_: pData_ is a always a
+        // pointer to a buffer owned by somebody else. Usually it is a
+        // pointer into a copy of the image file, but if
+        // TiffEntryBase::setData is used then it is a pointer into the
+        // storage_ DataBuf below.
         byte*    pData_;      //!< Pointer to the data area
-        bool     isMalloced_; //!< True if this entry owns the value data
+
         int      idx_;        //!< Unique id of the entry in the image
         Value*   pValue_;     //!< Converted data value
 
+        // This DataBuf is only used when TiffEntryBase::setData is called.
+        // Otherwise, it remains empty. It is wrapped in a shared_ptr because
+        // TiffEntryBase has a clone method, which could lead to the DataBuf
+        // having multiple owners.
+        std::shared_ptr<DataBuf> storage_;
     }; // class TiffEntryBase
 
     /*!
diff --git a/src/tiffvisitor_int.cpp b/src/tiffvisitor_int.cpp
index 1540cda3..a4a833b1 100644
--- a/src/tiffvisitor_int.cpp
+++ b/src/tiffvisitor_int.cpp
@@ -1653,8 +1653,10 @@ namespace Exiv2 {
         if (cryptFct != nullptr) {
             const byte* pData = object->pData();
             int32_t size = object->TiffEntryBase::doSize();
-            DataBuf buf = cryptFct(object->tag(), pData, size, pRoot_);
-            if (buf.size() > 0) object->setData(std::move(buf));
+            std::shared_ptr<DataBuf> buf = std::make_shared<DataBuf>(
+              cryptFct(object->tag(), pData, size, pRoot_)
+            );
+            if (buf->size() > 0) object->setData(buf);
         }
 
         const ArrayDef* defs = object->def();