Replace DataBuf internal buffer with std::vector

app/actions.cpp

@@ -62,6 +62,8 @@
 #ifdef _MSC_VER
 # include <sys/utime.h>
 #include <Windows.h>
+#include <fcntl.h>
+#include <io.h>
 #else
 # include <utime.h>
 #endif
@@ -1820,11 +1822,15 @@ namespace {
         bool          bStdout = tgt    == "-";
 
         Exiv2::DataBuf stdIn;
-        if ( bStdin )
+        Exiv2::Image::UniquePtr sourceImage;
+        if ( bStdin ) {
             Params::instance().getStdin(stdIn);
+            auto ioStdin = std::make_unique<Exiv2::MemIo>(stdIn.c_data(),stdIn.size());
+            sourceImage = Exiv2::ImageFactory::open(std::move(ioStdin));
+        } else {
+            sourceImage = Exiv2::ImageFactory::open(source);
+        }
 
-        auto ioStdin = std::make_unique<Exiv2::MemIo>(stdIn.c_data(),stdIn.size());
-        auto sourceImage = bStdin ? Exiv2::ImageFactory::open(std::move(ioStdin)) : Exiv2::ImageFactory::open(source);
 
         assert(sourceImage);
         sourceImage->readMetadata();

app/exiv2.cpp

@@ -37,11 +37,14 @@
 #include <cstring>
 #include <cassert>
 #include <cctype>
-
 #include <regex>
 
 #if defined(_MSC_VER)
 #include <Windows.h>
+#include <fcntl.h>
+#include <io.h>
+#else
+#include <unistd.h>
 #endif
 
 // *****************************************************************************

include/exiv2/basicio.hpp

@@ -764,6 +764,7 @@ namespace Exiv2 {
     class EXIV2API RemoteIo : public BasicIo {
     public:
         //! Destructor. Releases all managed memory.
+        RemoteIo();
         ~RemoteIo() override;
         //@}
 

include/exiv2/types.hpp

@@ -247,7 +247,7 @@ namespace Exiv2 {
       private:
         // DATA
         //! Pointer to the buffer, 0 if none has been allocated
-        byte* pData_;
+        std::vector<byte> pData_;
         //! The current size of the buffer
         long size_;
     }; // class DataBuf

src/basicio.cpp

@@ -1170,6 +1170,8 @@ namespace Exiv2 {
         delete[] blocksMap_;
     }
 
+    RemoteIo::RemoteIo() = default;
+
     RemoteIo::~RemoteIo()
     {
         if (p_) {

src/jpgimage.cpp

@@ -369,17 +369,16 @@ namespace Exiv2 {
         while (marker != sos_ && marker != eoi_ && search > 0) {
             // 2-byte buffer for reading the size.
             byte sizebuf[2];
-            uint16_t size = 0;
+            uint16_t size = 0; // Size of the segment, including the 2-byte size field
             if (markerHasLength(marker)) {
                 io_->readOrThrow(sizebuf, 2, kerFailedToReadImageData);
                 size = getUShort(sizebuf, bigEndian);
-                // `size` is the size of the segment, including the 2-byte size field
-                // that we just read.
                 enforce(size >= 2, kerFailedToReadImageData);
             }
 
             // Read the rest of the segment.
             DataBuf buf(size);
+            /// \todo check if it makes sense to check for size
             if (size > 0) {
                 io_->readOrThrow(buf.data(2), size - 2, kerFailedToReadImageData);
                 buf.copyBytes(0, sizebuf, 2);

src/pngchunk_int.cpp

@@ -154,7 +154,7 @@ namespace Exiv2
                 arr = DataBuf(text, textsize);
             } else if (type == iTXt_Chunk) {
                 enforce(data.size() >= Safe::add(keysize, 3), Exiv2::kerCorruptedMetadata);
-                const size_t nullCount = std::count(data.c_data(keysize + 3), data.c_data(data.size()), '\0');
+                const size_t nullCount = std::count(data.c_data(keysize + 3), data.c_data(data.size()-1), '\0');
                 enforce(nullCount >= nullSeparators, Exiv2::kerCorruptedMetadata);
 
                 // Extract a deflate compressed or uncompressed UTF-8 text chunk
@@ -277,7 +277,7 @@ namespace Exiv2
                     uint32_t sizeIptc = 0;
                     uint32_t sizeHdr = 0;
 
-                    const byte* pEnd = psData.c_data(psData.size());
+                    const byte* pEnd = psData.c_data(psData.size()-1);
                     const byte* pCur = psData.c_data();
                     while (pCur < pEnd && 0 == Photoshop::locateIptcIrb(pCur, static_cast<long>(pEnd - pCur), &record,
                                                                         &sizeHdr, &sizeIptc)) {
@@ -551,7 +551,7 @@ namespace Exiv2
             }
 
             const char* sp = text.c_str(1);             // current byte (space pointer)
-            const char* eot = text.c_str(text.size());  // end of text
+            const char* eot = text.c_str(text.size()-1);  // end of text
 
             if (sp >= eot) {
                 return DataBuf();

src/types.cpp

@@ -119,39 +119,38 @@ namespace Exiv2 {
     }
 
     DataBuf::DataBuf(DataBuf&& rhs)
-        : pData_(rhs.pData_), size_(rhs.size_)
+        : pData_(std::move(rhs.pData_)), size_(rhs.size_)
     {
-        rhs.pData_ = nullptr;
         rhs.size_ = 0;
     }
 
     DataBuf::~DataBuf()
-    { delete[] pData_; }
+    { }
 
-    DataBuf::DataBuf() : pData_(nullptr), size_(0)
+    DataBuf::DataBuf() : pData_(), size_(0)
     {}
 
-    DataBuf::DataBuf(long size) : pData_(new byte[size]()), size_(size)
+    DataBuf::DataBuf(long size) : pData_(size), size_(size)
     {}
 
-    DataBuf::DataBuf(const byte* pData, long size) : pData_(nullptr), size_(0)
+    DataBuf::DataBuf(const byte* pData, long size) : pData_(), size_(0)
     {
         if (size > 0) {
-            pData_ = new byte[size];
+            pData_.resize(size);
-            std::memcpy(pData_, pData, size);
+            std::memcpy(pData_.data(), pData, size);
             size_ = size;
         }
     }
 
     DataBuf::DataBuf(const DataBuf& rhs)
-        : DataBuf(rhs.pData_, rhs.size_)
+        : DataBuf(rhs.pData_.data(), rhs.size_)
     {}
 
     DataBuf& DataBuf::operator=(DataBuf&& rhs)
     {
-        if (this == &rhs) return *this;
+        if (this == &rhs)
-        reset();
+            return *this;
-        std::swap(pData_, rhs.pData_);
+        pData_ = std::move(rhs.pData_);
         std::swap(size_, rhs.size_);
         return *this;
     }
@@ -159,8 +158,7 @@ namespace Exiv2 {
     void DataBuf::alloc(long size)
     {
         if (size > size_) {
-            delete[] pData_;
+            pData_.resize(size);
-            pData_ = new byte[size];
             size_ = size;
         }
     }
@@ -168,11 +166,10 @@ namespace Exiv2 {
     void DataBuf::resize(long size)
     {
         if (size > size_) {
-            byte* newbuf = new byte[size];
+            std::vector<byte> newbuf(size);
             if (size_ > 0) {
-                memcpy(newbuf, pData_, size_);
+                memcpy(newbuf.data(), pData_.data(), size_);
             }
-            delete[] pData_;
             pData_ = newbuf;
         }
         size_ = size;
@@ -180,13 +177,12 @@ namespace Exiv2 {
 
     void DataBuf::reset()
     {
-        delete[] pData_;
+        pData_.clear();
-        pData_ = nullptr;
         size_ = 0;
     }
 
     void DataBuf::clear() {
-        memset(pData_, 0, size_);
+        memset(pData_.data(), 0, size_);
     }
 
     uint8_t Exiv2::DataBuf::read_uint8(size_t offset) const {
@@ -245,11 +241,13 @@ namespace Exiv2 {
         ull2Data(&pData_[offset], x, byteOrder);
     }
 
+    /// \todo do not use void*
     void Exiv2::DataBuf::copyBytes(size_t offset, const void* buf, size_t bufsize) {
         if (static_cast<size_t>(size_) < bufsize || offset > size_ - bufsize) {
             throw std::overflow_error("Overflow in Exiv2::DataBuf::copyBytes");
+        } if (bufsize > 0) {
+            memcpy(&pData_[offset], buf, bufsize);
         }
-        memcpy(&pData_[offset], buf, bufsize);
     }
 
     int Exiv2::DataBuf::cmpBytes(size_t offset, const void* buf, size_t bufsize) const {
@@ -262,6 +260,8 @@ namespace Exiv2 {
     byte* Exiv2::DataBuf::data(size_t offset) {
         if (static_cast<size_t>(size_) < offset) {
             throw std::overflow_error("Overflow in Exiv2::DataBuf::c_data");
+        } else if (size_ == 0 || static_cast<size_t>(size_) == offset) {
+            return nullptr;
         }
         return &pData_[offset];
     }
@@ -269,6 +269,8 @@ namespace Exiv2 {
     const byte* Exiv2::DataBuf::c_data(size_t offset) const {
         if (static_cast<size_t>(size_) < offset) {
             throw std::overflow_error("Overflow in Exiv2::DataBuf::c_data");
+        } else if (size_ == 0 || static_cast<size_t>(size_) == offset) {
+            return nullptr;
         }
         return &pData_[offset];
     }

tests/bugfixes/github/test_issue_428.py

@@ -26,12 +26,15 @@ class PngReadRawProfile(metaclass=system_tests.CaseMeta):
     commands = ["$exiv2 " + fname for fname in filenames]
     stdout = [""] * len(filenames)
     stderr = [ stderr_exception(fname) for fname in filenames[0:5] ]
+
     stderr.append("""$exiv2_exception_message """ + filenames[5] + """:
 $kerInputDataReadFailed
 """)
+
     stderr.append("""Error: XMP Toolkit error 201: Error in XMLValidator
 Warning: Failed to decode XMP metadata.
 """ + stderr_exception(filenames[6]))
+
     stderr.append("""Warning: Failed to decode Exif metadata.
 """ + stderr_exception(filenames[7]))