From 482cb2ded5002e673e58b64907a1b51ca0baf07e Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Sun, 26 Sep 2021 15:52:58 +0100 Subject: [PATCH] Clarify ownership model of CiffComponent::pData_ --- src/crwimage_int.cpp | 16 ++++------------ src/crwimage_int.hpp | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/crwimage_int.cpp b/src/crwimage_int.cpp index d007ad0e..71cb8347 100644 --- a/src/crwimage_int.cpp +++ b/src/crwimage_int.cpp @@ -176,9 +176,7 @@ namespace Exiv2 { } CiffComponent::~CiffComponent() - { - if (isAllocated_) delete[] pData_; - } + {} CiffDirectory::~CiffDirectory() { @@ -560,15 +558,9 @@ namespace Exiv2 { void CiffComponent::setValue(DataBuf buf) { - if (isAllocated_) { - delete[] pData_; - pData_ = nullptr; - size_ = 0; - } - isAllocated_ = true; - std::pair p = buf.release(); - pData_ = p.first; - size_ = p.second; + storage_ = buf; + pData_ = storage_.c_data(); + size_ = storage_.size(); if (size_ > 8 && dataLocation() == directoryData) { tag_ &= 0x3fff; } diff --git a/src/crwimage_int.hpp b/src/crwimage_int.hpp index 884a3ed0..97d2401a 100644 --- a/src/crwimage_int.hpp +++ b/src/crwimage_int.hpp @@ -87,12 +87,12 @@ namespace Exiv2 { //@{ //! Default constructor CiffComponent() - : dir_(0), tag_(0), size_(0), offset_(0), pData_(0), - isAllocated_(false) {} + : dir_(0), tag_(0), size_(0), offset_(0), pData_(0) + {} //! Constructor taking a tag and directory CiffComponent(uint16_t tag, uint16_t dir) - : dir_(dir), tag_(tag), size_(0), offset_(0), pData_(0), - isAllocated_(false) {} + : dir_(dir), tag_(tag), size_(0), offset_(0), pData_(0) + {} //! Virtual destructor. virtual ~CiffComponent(); //@} @@ -287,9 +287,17 @@ namespace Exiv2 { uint16_t tag_; //!< Tag of the entry uint32_t size_; //!< Size of the data area uint32_t offset_; //!< Offset to the data area from start of dir + + // Notes on the ownership model of pData_: pData_ is a always a read-only + // pointer to a buffer owned by somebody else. Usually it is a pointer + // into a copy of the image that was memory-mapped in CrwImage::readMetadata(). + // However, if CiffComponent::setValue() is used, then it is a pointer into + // the storage_ DataBuf below. const byte* pData_; //!< Pointer to the data area - bool isAllocated_; //!< True if this entry owns the value data + // This DataBuf is only used when CiffComponent::setValue is called. + // Otherwise, it remains empty. + DataBuf storage_; }; // class CiffComponent /*!