Merge pull request #1956 from kevinbackhouse/DataBufMoveCnstr

Use std::move to transfer ownership of DataBufs.

include/exiv2/image.hpp

@@ -222,7 +222,7 @@ namespace Exiv2 {
           @param iccProfile DataBuf containing profile (binary)
           @param bTestValid - tests that iccProfile contains credible data
          */
-        virtual void setIccProfile(DataBuf& iccProfile,bool bTestValid=true);
+        virtual void setIccProfile(DataBuf&& iccProfile,bool bTestValid=true);
         /*!
           @brief Erase iccProfile. the profile is not removed from
               the actual image until the writeMetadata() method is called.
@@ -240,7 +240,8 @@ namespace Exiv2 {
         /*!
           @brief return iccProfile
          */
-        virtual DataBuf* iccProfile() { return &iccProfile_; }
+        virtual const DataBuf& iccProfile() const { return iccProfile_; }
+
         /*!
           @brief Copy all existing metadata from source Image. The data is
               copied into internal buffers and is not written to the image

include/exiv2/preview.hpp

@@ -145,7 +145,7 @@ namespace Exiv2 {
 
     private:
         //! Private constructor
-        PreviewImage(PreviewProperties properties, DataBuf data);
+        PreviewImage(PreviewProperties properties, DataBuf&& data);
 
         PreviewProperties properties_;          //!< Preview image properties
         DataBuf preview_;                       //!< Preview image data

include/exiv2/types.hpp

@@ -153,18 +153,6 @@ namespace Exiv2 {
 
     };
 
-    /*!
-      @brief Auxiliary type to enable copies and assignments, similar to
-             std::unique_ptr_ref. See http://www.josuttis.com/libbook/auto_ptr.html
-             for a discussion.
-     */
-    struct EXIV2API DataBufRef {
-        //! Constructor
-        explicit DataBufRef(std::pair<byte*, long> rhs) : p(rhs) {}
-        //! Pointer to a byte array and its size
-        std::pair<byte*, long> p;
-    };
-
     /*!
       @brief Utility class containing a character array. All it does is to take
              care of memory allocation and deletion. Its primary use is meant to
@@ -181,11 +169,15 @@ namespace Exiv2 {
         //! Constructor, copies an existing buffer
         DataBuf(const byte* pData, long size);
         /*!
-          @brief Copy constructor. Transfers the buffer to the newly created
+          @brief Copy constructor. Copies an existing DataBuf.
+         */
+        DataBuf(const DataBuf& rhs);
+        /*!
+          @brief Move constructor. Transfers the buffer to the newly created
                  object similar to std::unique_ptr, i.e., the original object is
                  modified.
          */
-        DataBuf(DataBuf& rhs);
+        DataBuf(DataBuf&& rhs);
         //! Destructor, deletes the allocated buffer
         ~DataBuf();
         //@}
@@ -197,7 +189,11 @@ namespace Exiv2 {
                  buffer at the original object similar to std::unique_ptr, i.e.,
                  the original object is modified.
          */
-        DataBuf& operator=(DataBuf& rhs);
+        DataBuf& operator=(DataBuf&& rhs);
+
+        // No copy assignment.
+        DataBuf& operator=(const DataBuf&) = delete;
+
         /*!
           @brief Allocate a data buffer of at least the given size. Note that if
                  the requested \em size is less than the current buffer size, no
@@ -208,6 +204,7 @@ namespace Exiv2 {
           @brief Resize the buffer. Existing data is preserved (like std::realloc()).
          */
         void resize(long size);
+
         /*!
           @brief Release ownership of the buffer to the caller. Returns the
                  buffer as a data pointer and size pair, resets the internal
@@ -216,25 +213,12 @@ namespace Exiv2 {
         EXV_WARN_UNUSED_RESULT std::pair<byte*, long> release();
 
         //! Reset value
-        void reset(std::pair<byte*, long> = {nullptr, long(0)});
+        void reset();
         //@}
 
         //! Fill the buffer with zeros.
         void clear();
 
-        /*!
-          @name Conversions
-
-          Special conversions with auxiliary type to enable copies
-          and assignments, similar to those used for std::unique_ptr.
-          See http://www.josuttis.com/libbook/auto_ptr.html for a discussion.
-         */
-        //@{
-        DataBuf(const DataBufRef& rhs);
-        DataBuf& operator=(DataBufRef rhs);
-        operator DataBufRef();
-        //@}
-
         long size() const { return size_; }
 
         uint8_t read_uint8(size_t offset) const;

src/actions.cpp

@@ -1025,15 +1025,15 @@ namespace Action {
             } else {
 
                 if ( bStdout ) { // -eC-
-                    std::cout.write(image->iccProfile()->c_str(),
-                                    image->iccProfile()->size());
+                    std::cout.write(image->iccProfile().c_str(),
+                                    image->iccProfile().size());
                 } else {
                     if (Params::instance().verbose_) {
                         std::cout << _("Writing iccProfile: ") << target << std::endl;
                     }
                     Exiv2::FileIo iccFile(target);
                     iccFile.open("wb") ;
-                    iccFile.write(image->iccProfile()->c_data(),image->iccProfile()->size());
+                    iccFile.write(image->iccProfile().c_data(),image->iccProfile().size());
                     iccFile.close();
                 }
             }
@@ -1180,7 +1180,7 @@ namespace Action {
         if ( iccPath == "-" ) {
             Exiv2::DataBuf              iccProfile ;
             Params::instance().getStdin(iccProfile);
-            rc =  insertIccProfile(path,iccProfile);
+            rc =  insertIccProfile(path,std::move(iccProfile));
         } else {
             if (!Exiv2::fileExists(iccProfilePath, true)) {
                 std::cerr << iccProfilePath
@@ -1188,13 +1188,13 @@ namespace Action {
                 rc = -1;
             } else {
                 Exiv2::DataBuf iccProfile = Exiv2::readFile(iccPath);
-                rc = insertIccProfile(path,iccProfile);
+                rc = insertIccProfile(path,std::move(iccProfile));
             }
         }
         return rc;
     } // Insert::insertIccProfile
 
-    int Insert::insertIccProfile(const std::string& path, Exiv2::DataBuf& iccProfileBlob)
+    int Insert::insertIccProfile(const std::string& path, Exiv2::DataBuf&& iccProfileBlob)
     {
         int rc = 0;
         // test path exists
@@ -1211,7 +1211,7 @@ namespace Action {
             // clear existing profile, assign the blob and rewrite image
             image->clearIccProfile();
             if ( iccProfileBlob.size() ) {
-                image->setIccProfile(iccProfileBlob);
+                image->setIccProfile(std::move(iccProfileBlob));
             }
             image->writeMetadata();
         }

src/actions.hpp

@@ -360,7 +360,7 @@ namespace Action {
         /*!
           @brief Insert an ICC profile from binary DataBuf into file \em path.
          */
-        static int insertIccProfile(const std::string& path, Exiv2::DataBuf& iccProfileBlob);
+        static int insertIccProfile(const std::string& path, Exiv2::DataBuf&& iccProfileBlob);
 
     private:
         Insert* clone_() const override;

src/bmffimage.cpp

@@ -411,12 +411,12 @@ namespace Exiv2
                     skip+=4;
                     if ( colour_type == "rICC" || colour_type == "prof" ) {
                         DataBuf       profile(data.c_data(skip),data.size()-skip);
-                        setIccProfile(profile);
+                        setIccProfile(std::move(profile));
                     } else if ( meth == 2 && prec == 0 && approx == 0 ) {
                         // JP2000 files have a 3 byte head // 2 0 0 icc......
                         skip -= 1 ;
                         DataBuf       profile(data.c_data(skip),data.size()-skip);
-                        setIccProfile(profile);
+                        setIccProfile(std::move(profile));
                     }
                 }
             } break;

src/crwimage_int.cpp

@@ -553,9 +553,9 @@ namespace Exiv2 {
         }
     } // CiffDirectory::doPrint
 
-    void CiffComponent::setValue(DataBuf buf)
+    void CiffComponent::setValue(DataBuf&& buf)
     {
-        storage_ = buf;
+        storage_ = std::move(buf);
         pData_ = storage_.c_data();
         size_  = storage_.size();
         if (size_ > 8 && dataLocation() == directoryData) {
@@ -626,7 +626,7 @@ namespace Exiv2 {
         return nullptr;
     } // CiffDirectory::doFindComponent
 
-    void CiffHeader::add(uint16_t crwTagId, uint16_t crwDir, DataBuf buf)
+    void CiffHeader::add(uint16_t crwTagId, uint16_t crwDir, DataBuf&& buf)
     {
         CrwDirs crwDirs;
         CrwMap::loadStack(crwDirs, crwDir);
@@ -639,7 +639,7 @@ namespace Exiv2 {
         }
         CiffComponent* child = pRootDir_->add(crwDirs, crwTagId);
         if (child) {
-            child->setValue(buf);
+            child->setValue(std::move(buf));
         }
     }  // CiffHeader::add
 
@@ -1015,7 +1015,7 @@ namespace Exiv2 {
         if (ed != image.exifData().end()) {
             DataBuf buf(ed->size());
             ed->copy(buf.data(), pHead->byteOrder());
-            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, buf);
+            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, std::move(buf));
         }
         else {
             pHead->remove(pCrwMapping->crwTagId_, pCrwMapping->crwDir_);
@@ -1039,14 +1039,14 @@ namespace Exiv2 {
             DataBuf buf(size);
             buf.clear();
             buf.copyBytes(0, comment.data(), comment.size());
-            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, buf);
+            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, std::move(buf));
         }
         else {
             if (cc) {
                 // Just delete the value, do not remove the tag
                 DataBuf buf(cc->size());
                 buf.clear();
-                cc->setValue(buf);
+                cc->setValue(std::move(buf));
             }
         }
     } // CrwMap::encode0x0805
@@ -1079,7 +1079,7 @@ namespace Exiv2 {
                 pos += ed2->size();
             }
             assert(pos == size);
-            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, buf);
+            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, std::move(buf));
         }
         else {
             pHead->remove(pCrwMapping->crwTagId_, pCrwMapping->crwDir_);
@@ -1109,7 +1109,7 @@ namespace Exiv2 {
         if (buf.size() > 0) {
             // Write the number of shorts to the beginning of buf
             buf.write_uint16(0, static_cast<uint16_t>(buf.size()), pHead->byteOrder());
-            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, buf);
+            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, std::move(buf));
         }
         else {
             pHead->remove(pCrwMapping->crwTagId_, pCrwMapping->crwDir_);
@@ -1137,7 +1137,7 @@ namespace Exiv2 {
             DataBuf buf(12);
             buf.clear();
             buf.write_uint32(0, static_cast<uint32_t>(t), pHead->byteOrder());
-            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, buf);
+            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, std::move(buf));
         }
         else {
             pHead->remove(pCrwMapping->crwTagId_, pCrwMapping->crwDir_);
@@ -1183,7 +1183,7 @@ namespace Exiv2 {
                 d = RotationMap::degrees(static_cast<uint16_t>(edO->toLong()));
             }
             buf.write_uint32(12, d, pHead->byteOrder());
-            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, buf);
+            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, std::move(buf));
         }
         else {
             pHead->remove(pCrwMapping->crwTagId_, pCrwMapping->crwDir_);
@@ -1200,7 +1200,7 @@ namespace Exiv2 {
         ExifThumbC exifThumb(image.exifData());
         DataBuf buf = exifThumb.copy();
         if (buf.size() != 0) {
-            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, buf);
+            pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, std::move(buf));
         }
         else {
             pHead->remove(pCrwMapping->crwTagId_, pCrwMapping->crwDir_);

src/crwimage_int.hpp

@@ -164,7 +164,7 @@ namespace Exiv2 {
         //! Set the directory tag for this component.
         void setDir(uint16_t dir)       { dir_ = dir; }
         //! Set the data value of the entry.
-        void setValue(DataBuf buf);
+        void setValue(DataBuf&& buf);
         //@}
 
         //! Return the type id for a tag
@@ -446,7 +446,7 @@ namespace Exiv2 {
           @param crwDir   Parent directory of the tag.
           @param buf      Value to be set.
          */
-        void add(uint16_t crwTagId, uint16_t crwDir, DataBuf buf);
+        void add(uint16_t crwTagId, uint16_t crwDir, DataBuf&& buf);
         /*!
           @brief Remove entry \em crwTagId in directory \em crwDir from the parse
                  tree. If it's the last entry in the directory, the directory is

src/image.cpp

@@ -590,8 +590,8 @@ namespace Exiv2 {
         if (checkMode(mdIptc) & amWrite) {
             setIptcData(image.iptcData());
         }
-        if (checkMode(mdIccProfile) & amWrite && iccProfile()) {
-            setIccProfile(*iccProfile());
+        if (checkMode(mdIccProfile) & amWrite) {
+            setIccProfile(DataBuf(image.iccProfile()));
         }
         if (checkMode(mdXmp) & amWrite) {
             setXmpPacket(image.xmpPacket());
@@ -668,7 +668,7 @@ namespace Exiv2 {
         comment_ = comment;
     }
 
-    void Image::setIccProfile(Exiv2::DataBuf& iccProfile,bool bTestValid)
+    void Image::setIccProfile(Exiv2::DataBuf&& iccProfile,bool bTestValid)
     {
         if ( bTestValid ) {
             if (iccProfile.size() < static_cast<long>(sizeof(long))) {
@@ -679,7 +679,7 @@ namespace Exiv2 {
                 throw Error(kerInvalidIccProfile);
             }
         }
-        iccProfile_ = iccProfile;
+        iccProfile_ = std::move(iccProfile);
     }
 
     void Image::clearIccProfile()

src/jp2image.cpp

@@ -303,7 +303,7 @@ static void boxes_check(size_t b,size_t m)
                             }
                             std::cout << "Exiv2::Jp2Image::readMetadata: wrote iccProfile " << icc.size() << " bytes to " << iccPath << std::endl ;
 #endif
-                            setIccProfile(icc);
+                            setIccProfile(std::move(icc));
                         }
 
                         if( subBox.type == kJp2BoxTypeImageHeader)

src/jpgimage.cpp

@@ -496,7 +496,7 @@ namespace Exiv2 {
                     profile.copyBytes(0, iccProfile_.c_data(), iccProfile_.size());
                 }
                 profile.copyBytes(iccProfile_.size(), buf.c_data(2+14), icc_size);
-                setIccProfile(profile,chunk==chunks);
+                setIccProfile(std::move(profile),chunk==chunks);
             }
             else if (  pixelHeight_ == 0 && inRange2(marker,sof0_,sof3_,sof5_,sof15_) ) {
                 // We hit a SOFn (start-of-frame) marker

src/preview.cpp

@@ -1037,8 +1037,8 @@ namespace {
 // *****************************************************************************
 // class member definitions
 namespace Exiv2 {
-    PreviewImage::PreviewImage(PreviewProperties properties, DataBuf data)
-        : properties_(std::move(properties)), preview_(data)
+    PreviewImage::PreviewImage(PreviewProperties properties, DataBuf&& data)
+        : properties_(std::move(properties)), preview_(std::move(data))
     {}
 
     PreviewImage::PreviewImage(const PreviewImage &rhs)
@@ -1148,6 +1148,6 @@ namespace Exiv2 {
             buf = loader->getData();
         }
 
-        return PreviewImage(properties, buf);
+        return PreviewImage(properties, std::move(buf));
     }
 }                                       // namespace Exiv2

src/tiffvisitor_int.cpp

@@ -627,7 +627,7 @@ namespace Exiv2 {
                 buf.copyBytes(0, rawIptc.c_data(), rawIptc.size());
             }
             else {
-                buf = rawIptc; // Note: This resets rawIptc
+                buf = std::move(rawIptc); // Note: This resets rawIptc
             }
             value->read(buf.data(), buf.size(), byteOrder_);
             Exifdatum iptcDatum(iptcNaaKey, value.get());
@@ -1654,7 +1654,7 @@ namespace Exiv2 {
             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(buf);
+            if (buf.size() > 0) object->setData(std::move(buf));
         }
 
         const ArrayDef* defs = object->def();

src/types.cpp

@@ -121,11 +121,11 @@ namespace Exiv2 {
         return tit->size_;
     }
 
-    DataBuf::DataBuf(DataBuf& rhs)
+    DataBuf::DataBuf(DataBuf&& rhs)
         : pData_(rhs.pData_), size_(rhs.size_)
     {
-        std::pair<byte*, long> ret = rhs.release();
-        UNUSED(ret);
+        rhs.pData_ = nullptr;
+        rhs.size_ = 0;
     }
 
     DataBuf::~DataBuf()
@@ -146,10 +146,16 @@ namespace Exiv2 {
         }
     }
 
-    DataBuf& DataBuf::operator=(DataBuf& rhs)
+    DataBuf::DataBuf(const DataBuf& rhs)
+        : DataBuf(rhs.pData_, rhs.size_)
+    {}
+
+    DataBuf& DataBuf::operator=(DataBuf&& rhs)
     {
         if (this == &rhs) return *this;
-        reset(rhs.release());
+        reset();
+        std::swap(pData_, rhs.pData_);
+        std::swap(size_, rhs.size_);
         return *this;
     }
 
@@ -185,25 +191,17 @@ namespace Exiv2 {
         return p;
     }
 
-    void DataBuf::reset(std::pair<byte*, long> p)
+    void DataBuf::reset()
     {
-        if (pData_ != p.first) {
-            delete[] pData_;
-            pData_ = p.first;
-        }
-        size_ = p.second;
+        delete[] pData_;
+        pData_ = nullptr;
+        size_ = 0;
     }
 
     void DataBuf::clear() {
         memset(pData_, 0, size_);
     }
 
-    DataBuf::DataBuf(const DataBufRef &rhs) : pData_(rhs.p.first), size_(rhs.p.second) {}
-
-    DataBuf &DataBuf::operator=(DataBufRef rhs) { reset(rhs.p); return *this; }
-
-    Exiv2::DataBuf::operator DataBufRef() { return DataBufRef(release()); }
-
     uint8_t Exiv2::DataBuf::read_uint8(size_t offset) const {
         if (offset >= static_cast<size_t>(size_)) {
             throw std::overflow_error("Overflow in Exiv2::DataBuf::read_uint8");

src/webpimage.cpp

@@ -635,7 +635,7 @@ namespace Exiv2 {
                 pixelHeight_ = Exiv2::getULong(size_buf, littleEndian) + 1;
             } else if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_ICCP)) {
                 readOrThrow(*io_, payload.data(), payload.size(), Exiv2::kerCorruptedMetadata);
-                this->setIccProfile(payload);
+                this->setIccProfile(std::move(payload));
             } else if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_EXIF)) {
                 readOrThrow(*io_, payload.data(), payload.size(), Exiv2::kerCorruptedMetadata);