Merge pull request #1886 from kevinbackhouse/private-pData

Make fields of DataBuf private

fuzz/fuzz-read-print-write.cpp

@@ -17,7 +17,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t * data, size_t size) {
   try {
     Exiv2::DataBuf data_copy(data, size);
     Exiv2::Image::UniquePtr image =
-      Exiv2::ImageFactory::open(data_copy.pData_, size);
+      Exiv2::ImageFactory::open(data_copy.c_data(), size);
     assert(image.get() != 0);
 
     image->readMetadata();

include/exiv2/image.hpp

@@ -234,7 +234,7 @@ namespace Exiv2 {
          */
         virtual bool iccProfileDefined()
         {
-            return iccProfile_.size_ != 0;
+            return iccProfile_.size() != 0;
         }
 
         /*!

include/exiv2/preview.hpp

@@ -70,8 +70,6 @@ namespace Exiv2 {
         //@{
         //! Copy constructor
         PreviewImage(const PreviewImage& rhs);
-        //! Destructor.
-        ~PreviewImage();
         //@}
 
         //! @name Manipulators
@@ -150,8 +148,7 @@ namespace Exiv2 {
         PreviewImage(PreviewProperties properties, DataBuf data);
 
         PreviewProperties properties_;          //!< Preview image properties
-        byte* pData_;                           //!< Pointer to the preview image data
-        uint32_t size_;                         //!< Size of the preview image data
+        DataBuf preview_;                       //!< Preview image data
 
     }; // class PreviewImage
 

include/exiv2/types.hpp

@@ -204,6 +204,10 @@ namespace Exiv2 {
                  new memory is allocated and the buffer size doesn't change.
          */
         void alloc(long size);
+        /*!
+          @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
@@ -215,6 +219,9 @@ namespace Exiv2 {
         void reset(std::pair<byte*, long> = {nullptr, long(0)});
         //@}
 
+        //! Fill the buffer with zeros.
+        void clear();
+
         /*!
           @name Conversions
 
@@ -228,6 +235,36 @@ namespace Exiv2 {
         operator DataBufRef();
         //@}
 
+        long size() const { return size_; }
+
+        uint8_t read_uint8(size_t offset) const;
+        void write_uint8(size_t offset, uint8_t x);
+
+        uint16_t read_uint16(size_t offset, ByteOrder byteOrder) const;
+        void write_uint16(size_t offset, uint16_t x, ByteOrder byteOrder);
+
+        uint32_t read_uint32(size_t offset, ByteOrder byteOrder) const;
+        void write_uint32(size_t offset, uint32_t x, ByteOrder byteOrder);
+
+        uint64_t read_uint64(size_t offset, ByteOrder byteOrder) const;
+        void write_uint64(size_t offset, uint64_t x, ByteOrder byteOrder);
+
+        //! Copy bytes into the buffer (starting at address &pData_[offset]).
+        void copyBytes(size_t offset, const void* buf, size_t bufsize);
+
+        //! Equivalent to: memcmp(&pData_[offset], buf, bufsize)
+        int cmpBytes(size_t offset, const void* buf, size_t bufsize) const;
+
+        //! Returns a data pointer.
+        byte* data(size_t offset = 0);
+
+        //! Returns a (read-only) data pointer.
+        const byte* c_data(size_t offset = 0) const;
+
+        //! Returns a (read-only) C-style string pointer.
+        const char* c_str(size_t offset = 0) const;
+
+      private:
         // DATA
         //! Pointer to the buffer, 0 if none has been allocated
         byte* pData_;
@@ -302,6 +339,11 @@ namespace Exiv2 {
              return number of bytes written.
      */
     EXIV2API long ul2Data(byte* buf, uint32_t l, ByteOrder byteOrder);
+    /*!
+      @brief Convert an uint64_t to data, write the data to the buffer,
+             return number of bytes written.
+     */
+    EXIV2API long ull2Data(byte* buf, uint64_t l, ByteOrder byteOrder);
     /*!
       @brief Convert an unsigned rational to data, write the data to the buffer,
              return number of bytes written.

include/exiv2/webpimage.hpp

@@ -88,7 +88,7 @@ namespace Exiv2 {
         void doWriteMetadata(BasicIo& outIo);
         //! @name NOT Implemented
         //@{
-        static long getHeaderOffset(byte* data, long data_size, byte* header, long header_size);
+        static long getHeaderOffset(const byte* data, long data_size, const byte* header, long header_size);
         static bool equalsWebPTag(Exiv2::DataBuf& buf, const char* str);
         void debugPrintHex(byte *data, long size);
         void decodeChunks(long filesize);

samples/largeiptc-test.cpp

@@ -46,9 +46,9 @@ int main(int argc, char* const argv[])
             throw Exiv2::Error(Exiv2::kerDataSourceOpenFailed, io.path(), Exiv2::strError());
         }
         Exiv2::DataBuf buf(static_cast<long>(io.size()));
-        std::cout << "Reading " << buf.size_ << " bytes from " << data << "\n";
-        long readBytes = io.read(buf.pData_, buf.size_);
-        if (readBytes != buf.size_ || io.error() || io.eof()) {
+        std::cout << "Reading " << buf.size() << " bytes from " << data << "\n";
+        long readBytes = io.read(buf.data(), buf.size());
+        if (readBytes != buf.size() || io.error() || io.eof()) {
             throw Exiv2::Error(Exiv2::kerFailedToReadImageData);
         }
 
@@ -59,28 +59,28 @@ int main(int argc, char* const argv[])
 
         // Set Preview field to the content of the data file
         Exiv2::DataValue value;
-        value.read(buf.pData_, buf.size_);
+        value.read(buf.data(), buf.size());
         Exiv2::IptcData& iptcData = image->iptcData();
         std::cout << "IPTC fields: " << iptcData.size() << "\n";
         iptcData["Iptc.Application2.Preview"] = value;
         std::cout << "IPTC fields: " << iptcData.size() << "\n";
 
         // Set IRB, compare with IPTC raw data
         Exiv2::DataBuf irb = Exiv2::Photoshop::setIptcIrb(nullptr, 0, iptcData);
-        std::cout << "IRB buffer : " << irb.size_ << "\n";
+        std::cout << "IRB buffer : " << irb.size() << "\n";
         const Exiv2::byte* record;
         uint32_t sizeHdr = 0;
         uint32_t sizeData = 0;
-        Exiv2::Photoshop::locateIptcIrb(irb.pData_, irb.size_, &record, &sizeHdr, &sizeData);
+        Exiv2::Photoshop::locateIptcIrb(irb.data(), irb.size(), &record, &sizeHdr, &sizeData);
         Exiv2::DataBuf rawIptc = Exiv2::IptcParser::encode(iptcData);
         std::cout << "Comparing IPTC and IRB size... ";
-        if (static_cast<uint32_t>(rawIptc.size_) != sizeData) {
+        if (static_cast<uint32_t>(rawIptc.size()) != sizeData) {
             std::cout << "not ";
         }
         std::cout << "ok\n";
 
         std::cout << "Comparing IPTC and IRB data... ";
-        if (0 != memcmp(rawIptc.pData_, record + sizeHdr, sizeData)) {
+        if (0 != rawIptc.cmpBytes(0, record + sizeHdr, sizeData)) {
             std::cout << "not ";
         }
         std::cout << "ok\n";

samples/mmap-test.cpp

@@ -50,11 +50,11 @@ try {
     long size = static_cast<long>(file.size());
     DataBuf buf(size);
     // Read from the memory mapped region
-    memcpy(buf.pData_, pData, buf.size_);
+    buf.copyBytes(0, pData, buf.size());
     // Reopen file in write mode and write to it
-    file.write(buf.pData_, buf.size_);
+    file.write(buf.c_data(), buf.size());
     // Read from the mapped region again
-    memcpy(buf.pData_, pData, buf.size_);
+    buf.copyBytes(0, pData, buf.size());
     file.close();
 
     return 0;

samples/mrwthumb.cpp

@@ -56,12 +56,12 @@ int main(int argc, char* const argv[])
             Exiv2::DataBuf buf = format->dataArea();
 
             // The first byte of the buffer needs to be patched
-            buf.pData_[0] = 0xff;
+            buf.write_uint8(0, 0xff);
 
             Exiv2::FileIo file("img_thumb.jpg");
 
             file.open("wb");
-            file.write(buf.pData_, buf.size_);
+            file.write(buf.c_data(), buf.size());
             file.close();
         }
 

samples/tiff-test.cpp

@@ -73,7 +73,7 @@ void mini1(const char* path)
 
     // Write nothing, this time with a previous binary image
     DataBuf buf = readFile(path);
-    wm = ExifParser::encode(blob, buf.pData_, buf.size_, bigEndian, exifData);
+    wm = ExifParser::encode(blob, buf.c_data(), buf.size(), bigEndian, exifData);
     enforce(wm == wmIntrusive, Exiv2::kerErrorMessage, "encode returned an unexpected value");
     assert(blob.empty());
     std::cout << "Test 2: Writing empty Exif data with original binary data: ok.\n";

samples/xmpparse.cpp

@@ -39,7 +39,7 @@ try {
     }
     Exiv2::DataBuf buf = Exiv2::readFile(argv[1]);
     std::string xmpPacket;
-    xmpPacket.assign(reinterpret_cast<char*>(buf.pData_), buf.size_);
+    xmpPacket.assign(buf.c_str(), buf.size());
     Exiv2::XmpData xmpData;
     if (0 != Exiv2::XmpParser::decode(xmpData, xmpPacket)) {
         std::string error(argv[1]);

samples/xmpparser-test.cpp

@@ -40,7 +40,7 @@ try {
     std::string filename(argv[1]);
     Exiv2::DataBuf buf = Exiv2::readFile(filename);
     std::string xmpPacket;
-    xmpPacket.assign(reinterpret_cast<char*>(buf.pData_), buf.size_);
+    xmpPacket.assign(buf.c_str(), buf.size());
     std::cerr << "-----> Decoding XMP data read from " << filename << " <-----\n";
     Exiv2::XmpData xmpData;
     if (0 != Exiv2::XmpParser::decode(xmpData, xmpPacket)) {

src/actions.cpp

@@ -230,11 +230,11 @@ namespace Action {
                 size_t size = static_cast<long>(output.str().size());
                 Exiv2::DataBuf iccProfile(static_cast<long>(size));
                 Exiv2::DataBuf ascii(static_cast<long>(size * 3 + 1));
-                ascii.pData_[size * 3] = 0;
-                ::memcpy(iccProfile.pData_,output.str().c_str(),size);
-                if (Exiv2::base64encode(iccProfile.pData_, size, reinterpret_cast<char*>(ascii.pData_), size * 3)) {
+                ascii.write_uint8(size * 3, 0);
+                iccProfile.copyBytes(0,output.str().c_str(),size);
+                if (Exiv2::base64encode(iccProfile.c_data(), size, reinterpret_cast<char*>(ascii.data()), size * 3)) {
                     long       chunk = 60 ;
-                    std::string code = std::string("data:") + std::string(reinterpret_cast<char*>(ascii.pData_));
+                    std::string code = std::string("data:") + std::string(ascii.c_str());
                     long length = static_cast<long>(code.size());
                     for ( long start = 0 ; start < length ; start += chunk ) {
                         long   count = (start+chunk) < length ? chunk : length - start ;
@@ -330,12 +330,12 @@ namespace Action {
         }
         else {
             auto dataBuf = exifThumb.copy();
-            if (dataBuf.size_ == 0) {
+            if (dataBuf.size() == 0) {
                 std::cout << _("None");
             }
             else {
                 std::cout << exifThumb.mimeType() << ", "
-                          << dataBuf.size_ << " " << _("Bytes");
+                          << dataBuf.size() << " " << _("Bytes");
             }
         }
         std::cout << std::endl;
@@ -615,8 +615,8 @@ namespace Action {
                 std::cout << std::endl;
             first = false;
             Exiv2::DataBuf buf(md.size());
-            md.copy(buf.pData_, pImage->byteOrder());
-            Exiv2::hexdump(std::cout, buf.pData_, buf.size_);
+            md.copy(buf.data(), pImage->byteOrder());
+            Exiv2::hexdump(std::cout, buf.c_data(), buf.size());
         }
         std::cout << std::endl;
         return true;
@@ -955,9 +955,9 @@ namespace Action {
             if (dontOverwrite(thumbPath)) return 0;
             if (Params::instance().verbose_) {
                 Exiv2::DataBuf buf = exifThumb.copy();
-                if (buf.size_ != 0) {
+                if (buf.size() != 0) {
                     std::cout << _("Writing thumbnail") << " (" << exifThumb.mimeType() << ", "
-                              << buf.size_ << " " << _("Bytes") << ") " << _("to file") << " "
+                              << buf.size() << " " << _("Bytes") << ") " << _("to file") << " "
                               << thumbPath << std::endl;
                 }
             }
@@ -1025,15 +1025,15 @@ namespace Action {
             } else {
 
                 if ( bStdout ) { // -eC-
-                    std::cout.write(reinterpret_cast<const char*>(image->iccProfile()->pData_),
-                                    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()->pData_,image->iccProfile()->size_);
+                    iccFile.write(image->iccProfile()->c_data(),image->iccProfile()->size());
                     iccFile.close();
                 }
             }
@@ -1158,8 +1158,8 @@ namespace Action {
     int Insert::insertXmpPacket(const std::string& path, const Exiv2::DataBuf& xmpBlob, bool usePacket)
     {
         std::string xmpPacket;
-        for ( long i = 0 ; i < xmpBlob.size_ ; i++ ) {
-            xmpPacket += static_cast<char>(xmpBlob.pData_[i]);
+        for ( long i = 0 ; i < xmpBlob.size() ; i++ ) {
+            xmpPacket += static_cast<char>(xmpBlob.read_uint8(i));
         }
         Exiv2::Image::UniquePtr image = Exiv2::ImageFactory::open(path);
         assert(image.get() != 0);
@@ -1210,7 +1210,7 @@ namespace Action {
             image->readMetadata();
             // clear existing profile, assign the blob and rewrite image
             image->clearIccProfile();
-            if ( iccProfileBlob.size_ ) {
+            if ( iccProfileBlob.size() ) {
                 image->setIccProfile(iccProfileBlob);
             }
             image->writeMetadata();
@@ -1888,7 +1888,7 @@ namespace {
 
         Exiv2::DataBuf stdIn;
         if ( bStdin )  Params::instance().getStdin(stdIn);
-        Exiv2::BasicIo::UniquePtr ioStdin = Exiv2::BasicIo::UniquePtr(new Exiv2::MemIo(stdIn.pData_,stdIn.size_));
+        Exiv2::BasicIo::UniquePtr ioStdin = Exiv2::BasicIo::UniquePtr(new Exiv2::MemIo(stdIn.c_data(),stdIn.size()));
 
         Exiv2::Image::UniquePtr sourceImage = bStdin ? Exiv2::ImageFactory::open(std::move(ioStdin)) : Exiv2::ImageFactory::open(source);
         assert(sourceImage.get() != 0);