Merge pull request #2209 from Exiv2/mainTODOs

Improvements around TODO comments

Author: piponazo

app/actions.cpp

@@ -170,8 +170,8 @@ int setModeAndPrintStructure(Exiv2::PrintStructureOption option, const std::stri
   if (binary && option == Exiv2::kpsIccProfile) {
     std::stringstream output(std::stringstream::out | std::stringstream::binary);
     result = printStructure(output, option, path);
-    if (result == 0) {
-      std::string str = output.str();
+    std::string str = output.str();
+    if (result == 0 && !str.empty()) {
       Exiv2::DataBuf iccProfile(str.size());
       Exiv2::DataBuf ascii(str.size() * 3 + 1);
       ascii.write_uint8(str.size() * 3, 0);
@@ -549,9 +549,11 @@ bool Print::printMetadatum(const Exiv2::Metadatum& md, const Exiv2::Image* pImag
   if (Params::instance().printItems_ & Params::prHex) {
     if (!first)
       std::cout << std::endl;
-    Exiv2::DataBuf buf(md.size());
-    md.copy(buf.data(), pImage->byteOrder());
-    Exiv2::hexdump(std::cout, buf.c_data(), buf.size());
+    if (md.size() > 0) {
+      Exiv2::DataBuf buf(md.size());
+      md.copy(buf.data(), pImage->byteOrder());
+      Exiv2::hexdump(std::cout, buf.c_data(), buf.size());
+    }
   }
   std::cout << std::endl;
   return true;

include/exiv2/types.hpp

@@ -201,7 +201,7 @@ struct EXIV2API DataBuf {
   int cmpBytes(size_t offset, const void* buf, size_t bufsize) const;
 
   //! Returns a data pointer.
-  byte* data(size_t offset = 0);
+  [[nodiscard]] byte* data(size_t offset = 0);
 
   //! Returns a (read-only) data pointer.
   [[nodiscard]] const byte* c_data(size_t offset = 0) const;

samples/addmoddel.cpp

@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
+
 // Sample program showing how to add, modify and delete Exif metadata.
 
 #include <exiv2/exiv2.hpp>

src/crwimage_int.cpp

@@ -853,7 +853,7 @@ void CrwMap::encodeBasic(const Image& image, const CrwMapping* pCrwMapping, Ciff
   auto ed = image.exifData().findKey(ek);
 
   // Set the new value or remove the entry
-  if (ed != image.exifData().end()) {
+  if (ed != image.exifData().end() && ed->size() > 0) {
     DataBuf buf(ed->size());
     ed->copy(buf.data(), pHead->byteOrder());
     pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, std::move(buf));

src/epsimage.cpp

@@ -29,7 +29,7 @@
 // *****************************************************************************
 namespace {
 using namespace Exiv2;
-using Exiv2::byte;
+using namespace Exiv2::Internal;
 
 // signature of DOS EPS
 constexpr auto dosEpsSignature = std::string_view("\xC5\xD0\xD3\xC6");

src/jpgimage.cpp

@@ -184,8 +184,9 @@ void JpegBase::readMetadata() {
 #ifdef EXIV2_DEBUG_MESSAGES
       std::cerr << "Found app13 segment, size = " << size << "\n";
 #endif
-      // Append to psBlob
-      append(psBlob, buf.c_data(16), size - 16);
+      if (buf.size() > 16) {  // Append to psBlob
+        append(psBlob, buf.c_data(16), size - 16);
+      }
       // Check whether psBlob is complete
       if (!psBlob.empty() && Photoshop::valid(&psBlob[0], psBlob.size())) {
         --search;
@@ -745,9 +746,9 @@ void JpegBase::doWriteMetadata(BasicIo& outIo) {
           bo = littleEndian;
           setByteOrder(bo);
         }
-        WriteMethod wm = ExifParser::encode(blob, rawExif.c_data(), rawExif.size(), bo, exifData_);
         const byte* pExifData = rawExif.c_data();
         size_t exifSize = rawExif.size();
+        WriteMethod wm = ExifParser::encode(blob, pExifData, exifSize, bo, exifData_);
         if (wm == wmIntrusive) {
           pExifData = !blob.empty() ? &blob[0] : nullptr;
           exifSize = blob.size();
@@ -842,9 +843,9 @@ void JpegBase::doWriteMetadata(BasicIo& outIo) {
       if (foundCompletePsData || iptcData_.count() > 0) {
         // Set the new IPTC IRB, keeps existing IRBs but removes the
         // IPTC block if there is no new IPTC data to write
-        DataBuf newPsData = Photoshop::setIptcIrb(!psBlob.empty() ? psBlob.data() : nullptr, psBlob.size(), iptcData_);
+        DataBuf newPsData = Photoshop::setIptcIrb(psBlob.data(), psBlob.size(), iptcData_);
         const long maxChunkSize = 0xffff - 16;
-        const byte* chunkStart = newPsData.c_data();
+        const byte* chunkStart = newPsData.empty() ? nullptr : newPsData.c_data();
         const byte* chunkEnd = newPsData.empty() ? nullptr : newPsData.c_data(newPsData.size() - 1);
         while (chunkStart < chunkEnd) {
           // Determine size of next chunk

src/pngchunk_int.cpp

@@ -56,7 +56,8 @@ void PngChunk::decodeTXTChunk(Image* pImage, const DataBuf& data, TxtChunkType t
 #ifdef EXIV2_DEBUG_MESSAGES
   std::cout << "Exiv2::PngChunk::decodeTXTChunk: TXT chunk data: " << std::string(arr.c_str(), arr.size()) << std::endl;
 #endif
-  parseChunkContent(pImage, key.c_data(), key.size(), arr);
+  if (!key.empty())
+    parseChunkContent(pImage, key.c_data(), key.size(), arr);
 }
 
 DataBuf PngChunk::decodeTXTChunk(const DataBuf& data, TxtChunkType type) {
@@ -558,8 +559,10 @@ DataBuf PngChunk::readRawProfile(const DataBuf& text, bool iTXt) {
     return {};
   }
 
-  // Copy profile, skipping white space and column 1 "=" signs
+  if (info.empty())  // Early return
+    return info;
 
+  // Copy profile, skipping white space and column 1 "=" signs
   unsigned char* dp = info.data();  // decode pointer
   size_t nibbles = length * 2;
 

src/pngimage.cpp

@@ -18,6 +18,7 @@
 #include "pngimage.hpp"
 #include "tiffimage.hpp"
 #include "types.hpp"
+#include "utils.hpp"
 
 #include <array>
 #include <iostream>
@@ -171,14 +172,8 @@ static bool tEXtToDataBuf(const byte* bytes, long length, DataBuf& result) {
   return true;
 }
 
-std::string upper(const std::string& str) {
-  std::string result;
-  transform(str.begin(), str.end(), std::back_inserter(result), toupper);
-  return result;
-}
-
 std::string::size_type findi(const std::string& str, const std::string& substr) {
-  return upper(str).find(upper(substr));
+  return str.find(substr);
 }
 
 void PngImage::printStructure(std::ostream& out, PrintStructureOption option, int depth) {
@@ -194,14 +189,14 @@ void PngImage::printStructure(std::ostream& out, PrintStructureOption option, in
   chType[4] = 0;
 
   if (option == kpsBasic || option == kpsXMP || option == kpsIccProfile || option == kpsRecursive) {
-    constexpr auto xmpKey = "XML:com.adobe.xmp";
-    constexpr auto exifKey = "Raw profile type exif";
-    constexpr auto app1Key = "Raw profile type APP1";
-    constexpr auto iptcKey = "Raw profile type iptc";
-    constexpr auto iccKey = "icc";
-    constexpr auto softKey = "Software";
-    constexpr auto commKey = "Comment";
-    constexpr auto descKey = "Description";
+    const auto xmpKey = upper("XML:com.adobe.xmp");
+    const auto exifKey = upper("Raw profile type exif");
+    const auto app1Key = upper("Raw profile type APP1");
+    const auto iptcKey = upper("Raw profile type iptc");
+    const auto iccKey = upper("icc");
+    const auto softKey = upper("Software");
+    const auto commKey = upper("Comment");
+    const auto descKey = upper("Description");
 
     bool bPrint = option == kpsBasic || option == kpsRecursive;
     if (bPrint) {
@@ -234,8 +229,10 @@ void PngImage::printStructure(std::ostream& out, PrintStructureOption option, in
       }
 
       DataBuf buff(dataOffset);
-      bufRead = io_->read(buff.data(), dataOffset);
-      enforce(bufRead == dataOffset, ErrorCode::kerFailedToReadImageData);
+      if (dataOffset > 0) {
+        bufRead = io_->read(buff.data(), dataOffset);
+        enforce(bufRead == dataOffset, ErrorCode::kerFailedToReadImageData);
+      }
       io_->seek(restore, BasicIo::beg);
 
       // format output
@@ -273,15 +270,14 @@ void PngImage::printStructure(std::ostream& out, PrintStructureOption option, in
       bool eXIf = std::strcmp(chType, "eXIf") == 0;
 
       // for XMP, ICC etc: read and format data
-      /// \todo inside findi we are transforming the dataString to uppercase. Therefore we are transforming it
-      /// several times when we could do it just once and reuse it.
-      bool bXMP = option == kpsXMP && findi(dataString, xmpKey) == 0;
-      bool bICC = option == kpsIccProfile && findi(dataString, iccKey) == 0;
-      bool bExif = option == kpsRecursive && (findi(dataString, exifKey) == 0 || findi(dataString, app1Key) == 0);
-      bool bIptc = option == kpsRecursive && findi(dataString, iptcKey) == 0;
-      bool bSoft = option == kpsRecursive && findi(dataString, softKey) == 0;
-      bool bComm = option == kpsRecursive && findi(dataString, commKey) == 0;
-      bool bDesc = option == kpsRecursive && findi(dataString, descKey) == 0;
+      const auto dataStringU = upper(dataString);
+      bool bXMP = option == kpsXMP && findi(dataStringU, xmpKey) == 0;
+      bool bICC = option == kpsIccProfile && findi(dataStringU, iccKey) == 0;
+      bool bExif = option == kpsRecursive && (findi(dataStringU, exifKey) == 0 || findi(dataStringU, app1Key) == 0);
+      bool bIptc = option == kpsRecursive && findi(dataStringU, iptcKey) == 0;
+      bool bSoft = option == kpsRecursive && findi(dataStringU, softKey) == 0;
+      bool bComm = option == kpsRecursive && findi(dataStringU, commKey) == 0;
+      bool bDesc = option == kpsRecursive && findi(dataStringU, descKey) == 0;
       bool bDump = bXMP || bICC || bExif || bIptc || bSoft || bComm || bDesc || eXIf;
 
       if (bDump) {
@@ -426,7 +422,9 @@ void PngImage::readMetadata() {
     if (chunkType == "IEND" || chunkType == "IHDR" || chunkType == "tEXt" || chunkType == "zTXt" ||
         chunkType == "eXIf" || chunkType == "iTXt" || chunkType == "iCCP") {
       DataBuf chunkData(chunkLength);
-      readChunk(chunkData, *io_);  // Extract chunk data.
+      if (chunkLength > 0) {
+        readChunk(chunkData, *io_);  // Extract chunk data.
+      }
 
       if (chunkType == "IEND") {
         return;  // Last chunk found: we stop parsing.

src/rafimage.cpp

@@ -259,11 +259,11 @@ void RafImage::readMetadata() {
   if (io_->seek(jpg_img_off + 12, BasicIo::beg) != 0)
     throw Error(ErrorCode::kerFailedToReadImageData);
 
-  io_->read(buf.data(), buf.size());
-  if (io_->error() || io_->eof())
-    throw Error(ErrorCode::kerFailedToReadImageData);
-
-  io_->seek(0, BasicIo::beg);  // rewind
+  if (!buf.empty()) {
+    io_->read(buf.data(), buf.size());
+    if (io_->error() || io_->eof())
+      throw Error(ErrorCode::kerFailedToReadImageData);
+  }
 
   ByteOrder bo = TiffParser::decode(exifData_, iptcData_, xmpData_, buf.c_data(), buf.size());
 

src/tiffcomposite_int.cpp

@@ -976,7 +976,7 @@ uint32_t TiffDirectory::writeDirEntry(IoWrapper& ioWrapper, ByteOrder byteOrder,
 
 uint32_t TiffEntryBase::doWrite(IoWrapper& ioWrapper, ByteOrder byteOrder, int64_t /*offset*/, uint32_t /*valueIdx*/,
                                 uint32_t /*dataIdx*/, uint32_t& /*imageIdx*/) {
-  if (!pValue_)
+  if (!pValue_ || pValue_->size() == 0)
     return 0;
 
   DataBuf buf(pValue_->size());
@@ -1182,7 +1182,8 @@ uint32_t TiffDataEntry::doWriteData(IoWrapper& ioWrapper, ByteOrder /*byteOrder*
     return 0;
 
   DataBuf buf = pValue()->dataArea();
-  ioWrapper.write(buf.c_data(), buf.size());
+  if (!buf.empty())
+    ioWrapper.write(buf.c_data(), buf.size());
   // Align data to word boundary
   uint32_t align = (buf.size() & 1);
   if (align)