Merge pull request #2209 from Exiv2/mainTODOs

Improvements around TODO comments

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();
+    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,10 +549,12 @@ bool Print::printMetadatum(const Exiv2::Metadatum& md, const Exiv2::Image* pImag
   if (Params::instance().printItems_ & Params::prHex) {
     if (!first)
       std::cout << std::endl;
+    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;
 }  // Print::printMetadatum

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
+      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,6 +56,7 @@ 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
+  if (!key.empty())
     parseChunkContent(pImage, key.c_data(), key.size(), arr);
 }
 
@@ -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";
+    const auto xmpKey = upper("XML:com.adobe.xmp");
-    constexpr auto exifKey = "Raw profile type exif";
+    const auto exifKey = upper("Raw profile type exif");
-    constexpr auto app1Key = "Raw profile type APP1";
+    const auto app1Key = upper("Raw profile type APP1");
-    constexpr auto iptcKey = "Raw profile type iptc";
+    const auto iptcKey = upper("Raw profile type iptc");
-    constexpr auto iccKey = "icc";
+    const auto iccKey = upper("icc");
-    constexpr auto softKey = "Software";
+    const auto softKey = upper("Software");
-    constexpr auto commKey = "Comment";
+    const auto commKey = upper("Comment");
-    constexpr auto descKey = "Description";
+    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);
+      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
+      const auto dataStringU = upper(dataString);
-      /// several times when we could do it just once and reuse it.
+      bool bXMP = option == kpsXMP && findi(dataStringU, xmpKey) == 0;
-      bool bXMP = option == kpsXMP && findi(dataString, xmpKey) == 0;
+      bool bICC = option == kpsIccProfile && findi(dataStringU, iccKey) == 0;
-      bool bICC = option == kpsIccProfile && findi(dataString, iccKey) == 0;
+      bool bExif = option == kpsRecursive && (findi(dataStringU, exifKey) == 0 || findi(dataStringU, app1Key) == 0);
-      bool bExif = option == kpsRecursive && (findi(dataString, exifKey) == 0 || findi(dataString, app1Key) == 0);
+      bool bIptc = option == kpsRecursive && findi(dataStringU, iptcKey) == 0;
-      bool bIptc = option == kpsRecursive && findi(dataString, iptcKey) == 0;
+      bool bSoft = option == kpsRecursive && findi(dataStringU, softKey) == 0;
-      bool bSoft = option == kpsRecursive && findi(dataString, softKey) == 0;
+      bool bComm = option == kpsRecursive && findi(dataStringU, commKey) == 0;
-      bool bComm = option == kpsRecursive && findi(dataString, commKey) == 0;
+      bool bDesc = option == kpsRecursive && findi(dataStringU, descKey) == 0;
-      bool bDesc = option == kpsRecursive && findi(dataString, 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);
+      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);
 
+  if (!buf.empty()) {
     io_->read(buf.data(), buf.size());
     if (io_->error() || io_->eof())
       throw Error(ErrorCode::kerFailedToReadImageData);
-
+  }
-  io_->seek(0, BasicIo::beg);  // rewind
 
   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,6 +1182,7 @@ uint32_t TiffDataEntry::doWriteData(IoWrapper& ioWrapper, ByteOrder /*byteOrder*
     return 0;
 
   DataBuf buf = pValue()->dataArea();
+  if (!buf.empty())
     ioWrapper.write(buf.c_data(), buf.size());
   // Align data to word boundary
   uint32_t align = (buf.size() & 1);

src/types.cpp

@@ -110,85 +110,76 @@ void DataBuf::reset() {
 
 uint8_t Exiv2::DataBuf::read_uint8(size_t offset) const {
   if (offset >= pData_.size()) {
-    throw std::overflow_error("Overflow in Exiv2::DataBuf::read_uint8");
+    throw std::out_of_range("Overflow in Exiv2::DataBuf::read_uint8");
   }
   return pData_[offset];
 }
 
 void Exiv2::DataBuf::write_uint8(size_t offset, uint8_t x) {
   if (offset >= pData_.size()) {
-    throw std::overflow_error("Overflow in Exiv2::DataBuf::write_uint8");
+    throw std::out_of_range("Overflow in Exiv2::DataBuf::write_uint8");
   }
   pData_[offset] = x;
 }
 
 uint16_t Exiv2::DataBuf::read_uint16(size_t offset, ByteOrder byteOrder) const {
   if (pData_.size() < 2 || offset > (pData_.size() - 2)) {
-    throw std::overflow_error("Overflow in Exiv2::DataBuf::read_uint16");
+    throw std::out_of_range("Overflow in Exiv2::DataBuf::read_uint16");
   }
   return getUShort(&pData_[offset], byteOrder);
 }
 
 void Exiv2::DataBuf::write_uint16(size_t offset, uint16_t x, ByteOrder byteOrder) {
   if (pData_.size() < 2 || offset > (pData_.size() - 2)) {
-    throw std::overflow_error("Overflow in Exiv2::DataBuf::write_uint16");
+    throw std::out_of_range("Overflow in Exiv2::DataBuf::write_uint16");
   }
   us2Data(&pData_[offset], x, byteOrder);
 }
 
 uint32_t Exiv2::DataBuf::read_uint32(size_t offset, ByteOrder byteOrder) const {
   if (pData_.size() < 4 || offset > (pData_.size() - 4)) {
-    throw std::overflow_error("Overflow in Exiv2::DataBuf::read_uint32");
+    throw std::out_of_range("Overflow in Exiv2::DataBuf::read_uint32");
   }
   return getULong(&pData_[offset], byteOrder);
 }
 
 void Exiv2::DataBuf::write_uint32(size_t offset, uint32_t x, ByteOrder byteOrder) {
   if (pData_.size() < 4 || offset > (pData_.size() - 4)) {
-    throw std::overflow_error("Overflow in Exiv2::DataBuf::write_uint32");
+    throw std::out_of_range("Overflow in Exiv2::DataBuf::write_uint32");
   }
   ul2Data(&pData_[offset], x, byteOrder);
 }
 
 uint64_t Exiv2::DataBuf::read_uint64(size_t offset, ByteOrder byteOrder) const {
   if (pData_.size() < 8 || offset > (pData_.size() - 8)) {
-    throw std::overflow_error("Overflow in Exiv2::DataBuf::read_uint64");
+    throw std::out_of_range("Overflow in Exiv2::DataBuf::read_uint64");
   }
   return getULongLong(&pData_[offset], byteOrder);
 }
 
 void Exiv2::DataBuf::write_uint64(size_t offset, uint64_t x, ByteOrder byteOrder) {
   if (pData_.size() < 8 || offset > (pData_.size() - 8)) {
-    throw std::overflow_error("Overflow in Exiv2::DataBuf::write_uint64");
+    throw std::out_of_range("Overflow in Exiv2::DataBuf::write_uint64");
   }
   ull2Data(&pData_[offset], x, byteOrder);
 }
 
 int Exiv2::DataBuf::cmpBytes(size_t offset, const void* buf, size_t bufsize) const {
   if (pData_.size() < bufsize || offset > pData_.size() - bufsize) {
-    throw std::overflow_error("Overflow in Exiv2::DataBuf::cmpBytes");
+    throw std::out_of_range("Overflow in Exiv2::DataBuf::cmpBytes");
   }
   return memcmp(&pData_[offset], buf, bufsize);
 }
 
 byte* Exiv2::DataBuf::data(size_t offset) {
-  /// \todo this first check should be for <= offset
+  return const_cast<byte*>(c_data(offset));
-  if (pData_.size() < offset) {
-    throw std::overflow_error("Overflow in Exiv2::DataBuf::c_data");
-  }
-  if (pData_.empty() || pData_.size() == offset) {
-    return nullptr;
-  }
-  return &pData_[offset];
 }
 
 const byte* Exiv2::DataBuf::c_data(size_t offset) const {
-  /// \todo this first check should be for <= offset
+  if (pData_.empty()) {
-  if (pData_.size() < offset) {
-    throw std::overflow_error("Overflow in Exiv2::DataBuf::c_data");
-  }
-  if (pData_.empty() || pData_.size() == offset) {
     return nullptr;
+  } else if (offset >= pData_.size()) {
+    throw std::out_of_range("Overflow in Exiv2::DataBuf::c_data");
   }
   return &pData_[offset];
 }
@@ -220,7 +211,8 @@ std::ostream& operator<<(std::ostream& os, const Rational& r) {
   return os << r.first << "/" << r.second;
 }
 
-std::istream& operator>>(std::istream& is, Rational& r) {
+template <typename T>
+std::istream& fromStreamToRational(std::istream& is, T& r) {
   // http://dev.exiv2.org/boards/3/topics/1912?r=1915
   if (std::tolower(is.peek()) == 'f') {
     char F = 0;
@@ -241,30 +233,16 @@ std::istream& operator>>(std::istream& is, Rational& r) {
   return is;
 }
 
+std::istream& operator>>(std::istream& is, Rational& r) {
+  return fromStreamToRational(is, r);
+}
+
 std::ostream& operator<<(std::ostream& os, const URational& r) {
   return os << r.first << "/" << r.second;
 }
 
 std::istream& operator>>(std::istream& is, URational& r) {
-  // http://dev.exiv2.org/boards/3/topics/1912?r=1915
+  return fromStreamToRational(is, r);
-  /// \todo This implementation seems to be duplicated for the Rational type. Try to remove duplication
-  if (std::tolower(is.peek()) == 'f') {
-    char F = 0;
-    float f = 0.F;
-    is >> F >> f;
-    f = 2.0F * std::log(f) / std::log(2.0F);
-    r = Exiv2::floatToRationalCast(f);
-  } else {
-    uint32_t nominator = 0;
-    uint32_t denominator = 0;
-    char c('\0');
-    is >> nominator >> c >> denominator;
-    if (c != '/')
-      is.setstate(std::ios::failbit);
-    if (is)
-      r = {nominator, denominator};
-  }
-  return is;
 }
 
 uint16_t getUShort(const byte* buf, ByteOrder byteOrder) {
@@ -583,6 +561,7 @@ uint32_t parseUint32(const std::string& s, bool& ok) {
   if (ok && 0 <= x && x <= std::numeric_limits<uint32_t>::max()) {
     return static_cast<uint32_t>(x);
   }
+  ok = false;
   return 0;
 }
 
@@ -641,14 +620,12 @@ Rational floatToRationalCast(float f) {
   // Beware: primitive conversion algorithm
   int32_t den = 1000000;
   const auto d_as_long = static_cast<long>(d);
-  if (Safe::abs(d_as_long) > 2147) {
-    den = 10000;
-  }
-  if (Safe::abs(d_as_long) > 214748) {
-    den = 100;
-  }
   if (Safe::abs(d_as_long) > 21474836) {
     den = 1;
+  } else if (Safe::abs(d_as_long) > 214748) {
+    den = 100;
+  } else if (Safe::abs(d_as_long) > 2147) {
+    den = 10000;
   }
   const auto nom = static_cast<int32_t>(std::round(d * den));
   const int32_t g = gcd(nom, den);

src/utils.cpp

@@ -1,7 +1,21 @@
 #include "utils.hpp"
 
-namespace Exiv2 {
+#include <algorithm>
-bool startsWith(const std::string_view& s, const std::string_view& start) {
+#include <cctype>
-  return s.find(start) == 0;
+#include <iterator>
+
+namespace Exiv2::Internal {
+
+std::string upper(const std::string& str) {
+  std::string result;
+  std::transform(str.begin(), str.end(), std::back_inserter(result), ::toupper);
+  return result;
 }
-}  // namespace Exiv2
+
+std::string lower(const std::string& a) {
+  std::string b = a;
+  std::transform(a.begin(), a.end(), b.begin(), ::tolower);
+  return b;
+}
+
+}  // namespace Exiv2::Internal

src/utils.hpp

@@ -1,10 +1,21 @@
 #ifndef EXIV2_UTILS_HPP
 #define EXIV2_UTILS_HPP
 
+#include <string>
 #include <string_view>
 
-namespace Exiv2 {
+namespace Exiv2::Internal {
-bool startsWith(const std::string_view& s, const std::string_view& start);
+
+constexpr bool startsWith(std::string_view s, std::string_view start) {
+  return s.find(start) == 0;
 }
 
+/// @brief Returns the uppercase version of \b str
+std::string upper(const std::string& str);
+
+/// @brief Returns the lowercase version of \b str
+std::string lower(const std::string& str);
+
+}  // namespace Exiv2::Internal
+
 #endif  // EXIV2_UTILS_HPP

src/webpimage.cpp

@@ -162,11 +162,13 @@ void WebPImage::doWriteMetadata(BasicIo& outIo) {
     // Check that `size_u32` is safe to cast to `long`.
     enforce(size_u32 <= std::numeric_limits<uint32_t>::max(), Exiv2::ErrorCode::kerCorruptedMetadata);
     DataBuf payload(size_u32);
+    if (!payload.empty()) {
       io_->readOrThrow(payload.data(), payload.size(), Exiv2::ErrorCode::kerCorruptedMetadata);
       if (payload.size() % 2) {
         byte c = 0;
         io_->readOrThrow(&c, 1, Exiv2::ErrorCode::kerCorruptedMetadata);
       }
+    }
 
     /* Chunk with information about features
      used in the file. */
@@ -528,7 +530,9 @@ void WebPImage::decodeChunks(long filesize) {
 
     DataBuf payload(size);
 
-    if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8X) && !has_canvas_data) {
+    if (payload.empty()) {
+      io_->seek(size, BasicIo::cur);
+    } else if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8X) && !has_canvas_data) {
       enforce(size >= 10, Exiv2::ErrorCode::kerCorruptedMetadata);
 
       has_canvas_data = true;

src/xmpsidecar.cpp

@@ -7,6 +7,7 @@
 #include "error.hpp"
 #include "futils.hpp"
 #include "image.hpp"
+#include "utils.hpp"
 #include "xmp_exiv2.hpp"
 
 #include <iostream>
@@ -82,16 +83,8 @@ void XmpSidecar::readMetadata() {
   copyXmpToExif(xmpData_, exifData_);
 }  // XmpSidecar::readMetadata
 
-// lower case string
-/// \todo very similar function in pngimage (upper). We should move those things to a string utilities file
-static std::string toLowerCase(const std::string& a) {
-  std::string b = a;
-  std::transform(a.begin(), a.end(), b.begin(), ::tolower);
-  return b;
-}
-
 static bool matchi(const std::string& key, const char* substr) {
-  return toLowerCase(key).find(substr) != std::string::npos;
+  return Internal::lower(key).find(substr) != std::string::npos;
 }
 
 void XmpSidecar::writeMetadata() {

tests/bugfixes/github/test_issue_960.py

@@ -14,8 +14,7 @@ class WebPImageGetHeaderOffset(metaclass=CaseMeta):
     commands = ["$exiv2 $filename1"]
     stdout = [""]
     stderr = [
-"""Warning: Failed to decode Exif metadata.
+"""Exiv2 exception in print action for file $filename1:
-Exiv2 exception in print action for file $filename1:
 $kerCorruptedMetadata
 """
 ]

unitTests/CMakeLists.txt

@@ -25,6 +25,7 @@ add_executable(unit_tests
     test_tiffheader.cpp
     test_types.cpp
     test_TimeValue.cpp
+    test_utils.cpp
     test_XmpKey.cpp
     $<TARGET_OBJECTS:exiv2lib_int>
 )

unitTests/test_types.cpp

@@ -1,9 +1,14 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
+#include <exiv2/types.hpp>
+
 #include <gtest/gtest.h>
+
+#include <algorithm>
+#include <array>
 #include <cmath>
-#include <exiv2/types.hpp>
 #include <limits>
+
 using namespace Exiv2;
 
 // More info about tm : http://www.cplusplus.com/reference/ctime/tm/
@@ -24,18 +29,73 @@ TEST(ExivTime, doesNotGetTimeWithBadFormedString) {
   ASSERT_EQ(1, exifTime("007:a5:24 aa:bb:cc", &tmInstance));
 }
 
-TEST(DataBuf, pointsToNullByDefault) {
+TEST(DataBuf, defaultInstanceIsEmpty) {
   DataBuf instance;
-  ASSERT_EQ(nullptr, instance.c_data());
+  ASSERT_TRUE(instance.empty());
-  ASSERT_EQ(0, instance.size());
 }
 
 TEST(DataBuf, allocatesDataWithNonEmptyConstructor) {
   DataBuf instance(5);
-  ASSERT_NE(static_cast<byte*>(nullptr), instance.c_data());  /// \todo use nullptr once we move to c++11
+  ASSERT_NE(nullptr, instance.c_data());
   ASSERT_EQ(5, instance.size());
 }
 
+TEST(DataBuf, canBeConstructedFromExistingData) {
+  const std::array<byte, 4> data{'h', 'o', 'l', 'a'};
+  DataBuf instance(data.data(), data.size());
+  ASSERT_TRUE(std::equal(data.begin(), data.end(), instance.begin()));
+}
+
+TEST(DataBuf, tryingToAccessTooFarElementThrows) {
+  const std::array<byte, 4> data{'h', 'o', 'l', 'a'};
+  DataBuf instance(data.data(), data.size());
+  ASSERT_THROW([[maybe_unused]] auto d = instance.data(4), std::out_of_range);
+  ASSERT_THROW([[maybe_unused]] auto d = instance.c_data(4), std::out_of_range);
+}
+
+TEST(DataBuf, read_uintFunctionsWorksOnExistingData) {
+  const std::array<byte, 8> data{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07};
+  DataBuf instance(data.data(), data.size());
+  ASSERT_EQ(data[0], instance.read_uint8(0));
+  ASSERT_EQ(data[1], instance.read_uint16(0, bigEndian));
+  ASSERT_EQ(0x00010203, instance.read_uint32(0, bigEndian));
+  ASSERT_EQ(0x0001020304050607, instance.read_uint64(0, bigEndian));
+}
+
+TEST(DataBuf, read_uintFunctionsThrowsOnTooFarElements) {
+  const std::array<byte, 8> data{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07};
+  DataBuf instance(data.data(), data.size());
+  ASSERT_THROW([[maybe_unused]] auto d = instance.read_uint8(data.size()), std::out_of_range);
+  ASSERT_THROW([[maybe_unused]] auto d = instance.read_uint16(data.size(), bigEndian), std::out_of_range);
+  ASSERT_THROW([[maybe_unused]] auto d = instance.read_uint32(data.size(), bigEndian), std::out_of_range);
+  ASSERT_THROW([[maybe_unused]] auto d = instance.read_uint64(data.size(), bigEndian), std::out_of_range);
+}
+
+TEST(DataBuf, write_uintFunctionsWorksWhenThereIsEnoughData) {
+  DataBuf instance(8);
+  std::uint64_t val{0x0102030405060708};
+  ASSERT_NO_THROW(instance.write_uint8(0, (val >> 56)));
+  ASSERT_EQ(0x01, instance.read_uint8(0));
+
+  ASSERT_NO_THROW(instance.write_uint16(0, (val >> 48), bigEndian));
+  ASSERT_EQ(0x0102, instance.read_uint16(0, bigEndian));
+
+  ASSERT_NO_THROW(instance.write_uint32(0, (val >> 32), bigEndian));
+  ASSERT_EQ(0x01020304, instance.read_uint32(0, bigEndian));
+
+  ASSERT_NO_THROW(instance.write_uint64(0, val, bigEndian));
+  ASSERT_EQ(val, instance.read_uint64(0, bigEndian));
+}
+
+TEST(DataBuf, write_uintFunctionsThrowsIfTryingToWriteOutOfBounds) {
+  DataBuf instance(8);
+  std::uint64_t val{0x0102030405060708};
+  ASSERT_THROW(instance.write_uint8(8, (val >> 56)), std::out_of_range);
+  ASSERT_THROW(instance.write_uint16(7, (val >> 48), bigEndian), std::out_of_range);
+  ASSERT_THROW(instance.write_uint32(5, (val >> 32), bigEndian), std::out_of_range);
+  ASSERT_THROW(instance.write_uint64(1, (val >> 32), bigEndian), std::out_of_range);
+}
+
 // Test methods like DataBuf::read_uint32 and DataBuf::write_uint32.
 TEST(DataBuf, read_write_endianess) {
   DataBuf buf(4 + 1 + 2 + 4 + 8);
@@ -84,3 +144,103 @@ TEST(Rational, floatToRationalCast) {
   ASSERT_EQ(minus_inf.first, -1);
   ASSERT_EQ(minus_inf.second, 0);
 }
+
+TEST(Rational, toStream) {
+  Rational r = {1, 2};
+  std::stringstream str;
+  str << r;
+  ASSERT_EQ("1/2", str.str());
+}
+
+TEST(Rational, readRationalFromStream) {
+  Rational r;
+  std::istringstream input("1/2");
+  input >> r;
+  ASSERT_EQ(1, r.first);
+  ASSERT_EQ(2, r.second);
+}
+
+TEST(Rational, parseRationalFromStringSuccessfully) {
+  bool ok{false};
+  Rational rational = parseRational("1/2", ok);
+  ASSERT_EQ(std::make_pair(1, 2), rational);
+  ASSERT_TRUE(ok);
+}
+
+TEST(Rational, parseRationalFromLongIsOK) {
+  bool ok{true};
+  Rational rational = parseRational("12", ok);
+  ASSERT_EQ(std::make_pair(12, 1), rational);
+  ASSERT_TRUE(ok);
+}
+
+TEST(Rational, parseRationalFromBoolIsOK) {
+  bool ok{true};
+  Rational rational = parseRational("true", ok);
+  ASSERT_EQ(std::make_pair(1, 1), rational);
+  ASSERT_TRUE(ok);
+
+  rational = parseRational("false", ok);
+  ASSERT_EQ(std::make_pair(0, 1), rational);
+  ASSERT_TRUE(ok);
+}
+
+TEST(Rational, parseRationalFromFloatIsOK) {
+  bool ok{true};
+  Rational rational = parseRational("1.2", ok);
+  ASSERT_EQ(std::make_pair(6, 5), rational);
+  ASSERT_TRUE(ok);
+
+  rational = parseRational("1.4", ok);
+  ASSERT_EQ(std::make_pair(7, 5), rational);
+  ASSERT_TRUE(ok);
+}
+
+TEST(Rational, parseRationalFromFloatWithFCharIsNoOK) {
+  bool ok{true};
+  Rational rational = parseRational("1.2f", ok);
+  ASSERT_EQ(std::make_pair(0, 0), rational);
+  ASSERT_FALSE(ok);
+}
+
+TEST(Rational, floatToRationalCastWorks) {
+  ASSERT_EQ(std::make_pair(6, 5), floatToRationalCast(1.2f));
+  ASSERT_EQ(std::make_pair(11001, 5), floatToRationalCast(2200.2f));
+  ASSERT_EQ(std::make_pair(1100001, 5), floatToRationalCast(220000.2f));
+  ASSERT_EQ(std::make_pair(22000000, 1), floatToRationalCast(22000000.2f));
+  ASSERT_EQ(std::make_pair(22000000, 1), floatToRationalCast(22000000.2f));
+}
+
+TEST(Rational, floatToRationalCastWithIntegersOutOfLimits) {
+  ASSERT_EQ(std::make_pair(1, 0), floatToRationalCast(2247483647.f));
+  ASSERT_EQ(std::make_pair(-1, 0), floatToRationalCast(-2247483647.f));
+}
+
+TEST(URational, toStream) {
+  URational r = {1, 2};
+  std::stringstream str;
+  str << r;
+  ASSERT_EQ("1/2", str.str());
+}
+
+TEST(URational, readRationalFromStream) {
+  URational r;
+  std::istringstream input("1/2");
+  input >> r;
+  ASSERT_EQ(1, r.first);
+  ASSERT_EQ(2, r.second);
+}
+
+// --------------------
+
+TEST(parseUint32, withNumberInRangeReturnsOK) {
+  bool ok{false};
+  ASSERT_EQ(123456, parseUint32("123456", ok));
+  ASSERT_TRUE(ok);
+}
+
+TEST(parseUint32, withNumberOutOfRangeReturnsFalse) {
+  bool ok{false};
+  ASSERT_EQ(0, parseUint32("4333333333", ok));
+  ASSERT_FALSE(ok);
+}

unitTests/test_utils.cpp

@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include "utils.hpp"
+
+#include <gtest/gtest.h>
+
+using namespace Exiv2::Internal;
+
+TEST(stringUtils, startsWithReturnsTrue) {
+  ASSERT_TRUE(startsWith("Exiv2 rocks", "Exiv2"));
+}
+
+TEST(stringUtils, startsWithReturnsFlase) {
+  ASSERT_FALSE(startsWith("Exiv2 rocks", "exiv2"));
+}
+
+TEST(stringUtils, upperTransformStringToUpperCase) {
+  ASSERT_EQ("EXIV2 ROCKS", upper("Exiv2 rocks"));
+}
+
+TEST(stringUtils, lowerTransformStringToLowerCase) {
+  ASSERT_EQ("exiv2 rocks", lower("EXIV2 ROCKS"));
+}