From 4a4a8c544c340fc5bad998c9873126cb50f45e98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Tue, 12 Apr 2022 20:20:52 +0200 Subject: [PATCH 01/13] Throwing when trying to access TooFar elements in DataBuf --- src/types.cpp | 10 ++++------ unitTests/test_types.cpp | 23 +++++++++++++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/types.cpp b/src/types.cpp index a53f3c27..1d03c8b4 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -172,9 +172,8 @@ int Exiv2::DataBuf::cmpBytes(size_t offset, const void* buf, size_t bufsize) con } byte* Exiv2::DataBuf::data(size_t offset) { - /// \todo this first check should be for <= offset - if (pData_.size() < offset) { - throw std::overflow_error("Overflow in Exiv2::DataBuf::c_data"); + if (offset >= pData_.size()) { + throw std::out_of_range("Overflow in Exiv2::DataBuf::c_data"); } if (pData_.empty() || pData_.size() == offset) { return nullptr; @@ -183,9 +182,8 @@ byte* Exiv2::DataBuf::data(size_t offset) { } const byte* Exiv2::DataBuf::c_data(size_t offset) const { - /// \todo this first check should be for <= offset - if (pData_.size() < offset) { - throw std::overflow_error("Overflow in Exiv2::DataBuf::c_data"); + if (offset >= pData_.size()) { + throw std::out_of_range("Overflow in Exiv2::DataBuf::c_data"); } if (pData_.empty() || pData_.size() == offset) { return nullptr; diff --git a/unitTests/test_types.cpp b/unitTests/test_types.cpp index 72073304..227a345e 100644 --- a/unitTests/test_types.cpp +++ b/unitTests/test_types.cpp @@ -1,9 +1,13 @@ // SPDX-License-Identifier: GPL-2.0-or-later +#include + #include + +#include #include -#include #include + using namespace Exiv2; // More info about tm : http://www.cplusplus.com/reference/ctime/tm/ @@ -24,10 +28,9 @@ 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_EQ(0, instance.size()); + ASSERT_TRUE(instance.empty()); } TEST(DataBuf, allocatesDataWithNonEmptyConstructor) { @@ -36,6 +39,18 @@ TEST(DataBuf, allocatesDataWithNonEmptyConstructor) { ASSERT_EQ(5, instance.size()); } +TEST(DataBuf, canBeConstructedFromExistingData) { + const std::array 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 data {'h', 'o', 'l', 'a'}; + DataBuf instance(data.data(), data.size()); + ASSERT_THROW(instance.data(4), 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); From c3d0100d48e05b0c9e95c5ff032d46a0e91eb303 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Tue, 12 Apr 2022 20:21:18 +0200 Subject: [PATCH 02/13] Fixing bad usages of DataBuf (when it is empty) --- app/actions.cpp | 12 +++++++----- samples/addmoddel.cpp | 1 + src/bmffimage.cpp | 2 +- src/crwimage_int.cpp | 2 +- src/jpgimage.cpp | 11 ++++++----- src/pngchunk_int.cpp | 7 +++++-- src/pngimage.cpp | 10 +++++++--- src/rafimage.cpp | 12 +++++++----- src/tiffcomposite_int.cpp | 5 +++-- src/webpimage.cpp | 14 +++++++++----- tests/bugfixes/github/test_issue_960.py | 3 +-- 11 files changed, 48 insertions(+), 31 deletions(-) diff --git a/app/actions.cpp b/app/actions.cpp index 599a9534..439ec289 100644 --- a/app/actions.cpp +++ b/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; diff --git a/samples/addmoddel.cpp b/samples/addmoddel.cpp index c91204ae..0ea379d8 100644 --- a/samples/addmoddel.cpp +++ b/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 diff --git a/src/bmffimage.cpp b/src/bmffimage.cpp index a45076c0..977e1be8 100644 --- a/src/bmffimage.cpp +++ b/src/bmffimage.cpp @@ -620,7 +620,7 @@ void BmffImage::printStructure(std::ostream& out, Exiv2::PrintStructureOption op break; // do nothing case kpsIccProfile: { - out.write(iccProfile_.c_str(), iccProfile_.size()); + out.write(iccProfile_.empty() ? nullptr : iccProfile_.c_str(), iccProfile_.size()); } break; #ifdef EXV_HAVE_XMP_TOOLKIT diff --git a/src/crwimage_int.cpp b/src/crwimage_int.cpp index 01a39c55..ebc7e7e8 100644 --- a/src/crwimage_int.cpp +++ b/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)); diff --git a/src/jpgimage.cpp b/src/jpgimage.cpp index a191ff2b..57ce060c 100644 --- a/src/jpgimage.cpp +++ b/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(); + const byte* pExifData = rawExif.empty() ? nullptr : 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(); @@ -844,7 +845,7 @@ void JpegBase::doWriteMetadata(BasicIo& outIo) { // IPTC block if there is no new IPTC data to write DataBuf newPsData = Photoshop::setIptcIrb(!psBlob.empty() ? psBlob.data() : nullptr, 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 diff --git a/src/pngchunk_int.cpp b/src/pngchunk_int.cpp index 4e18431b..0855db9c 100644 --- a/src/pngchunk_int.cpp +++ b/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; diff --git a/src/pngimage.cpp b/src/pngimage.cpp index c7d274bc..c801bdd7 100644 --- a/src/pngimage.cpp +++ b/src/pngimage.cpp @@ -234,8 +234,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 @@ -426,7 +428,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. diff --git a/src/rafimage.cpp b/src/rafimage.cpp index f535f99d..0b048c45 100644 --- a/src/rafimage.cpp +++ b/src/rafimage.cpp @@ -259,13 +259,15 @@ 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); + if (!buf.empty()) { + io_->read(buf.data(), buf.size()); + if (io_->error() || io_->eof()) + throw Error(ErrorCode::kerFailedToReadImageData); + } - io_->seek(0, BasicIo::beg); // rewind + // io_->seek(0, BasicIo::beg); // rewind - ByteOrder bo = TiffParser::decode(exifData_, iptcData_, xmpData_, buf.c_data(), buf.size()); + ByteOrder bo = TiffParser::decode(exifData_, iptcData_, xmpData_, buf.empty() ? nullptr : buf.c_data(), buf.size()); exifData_["Exif.Image2.JPEGInterchangeFormat"] = getULong(jpg_img_offset, bigEndian); exifData_["Exif.Image2.JPEGInterchangeFormatLength"] = getULong(jpg_img_length, bigEndian); diff --git a/src/tiffcomposite_int.cpp b/src/tiffcomposite_int.cpp index af3d6a53..eb1d3c60 100644 --- a/src/tiffcomposite_int.cpp +++ b/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) diff --git a/src/webpimage.cpp b/src/webpimage.cpp index 71fdd599..3ed1dbaa 100644 --- a/src/webpimage.cpp +++ b/src/webpimage.cpp @@ -162,10 +162,12 @@ void WebPImage::doWriteMetadata(BasicIo& outIo) { // Check that `size_u32` is safe to cast to `long`. enforce(size_u32 <= std::numeric_limits::max(), Exiv2::ErrorCode::kerCorruptedMetadata); DataBuf payload(size_u32); - io_->readOrThrow(payload.data(), payload.size(), Exiv2::ErrorCode::kerCorruptedMetadata); - if (payload.size() % 2) { - byte c = 0; - io_->readOrThrow(&c, 1, Exiv2::ErrorCode::kerCorruptedMetadata); + 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 @@ -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; diff --git a/tests/bugfixes/github/test_issue_960.py b/tests/bugfixes/github/test_issue_960.py index 635ee3f7..ab5e19dc 100644 --- a/tests/bugfixes/github/test_issue_960.py +++ b/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 """ ] From 6964f5f9f2f83db92d94287c0f0d380db7006585 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Wed, 13 Apr 2022 13:25:50 +0200 Subject: [PATCH 03/13] DataBuf::c_data() returns nullptr when empty + avoid duplication After the previous commit, I realized that std::vector::data() also returns nullptr when the vector is empty. So I decided to emulate this behavior in DataBuf::c_data(). Anyways, the changes done in the previous commit are valid and allow us to avoid some processing when the DataBuf is empty. --- include/exiv2/types.hpp | 2 +- src/types.cpp | 15 ++++----------- unitTests/test_types.cpp | 3 ++- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/include/exiv2/types.hpp b/include/exiv2/types.hpp index 03419f0f..05268dac 100644 --- a/include/exiv2/types.hpp +++ b/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; diff --git a/src/types.cpp b/src/types.cpp index 1d03c8b4..a3f691e9 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -172,21 +172,14 @@ int Exiv2::DataBuf::cmpBytes(size_t offset, const void* buf, size_t bufsize) con } byte* Exiv2::DataBuf::data(size_t offset) { - if (offset >= pData_.size()) { - throw std::out_of_range("Overflow in Exiv2::DataBuf::c_data"); - } - if (pData_.empty() || pData_.size() == offset) { - return nullptr; - } - return &pData_[offset]; + return const_cast(c_data(offset)); } const byte* Exiv2::DataBuf::c_data(size_t offset) const { - if (offset >= pData_.size()) { - throw std::out_of_range("Overflow in Exiv2::DataBuf::c_data"); - } - if (pData_.empty() || pData_.size() == offset) { + if (pData_.empty()) { return nullptr; + } else if (offset >= pData_.size()) { + throw std::out_of_range("Overflow in Exiv2::DataBuf::c_data"); } return &pData_[offset]; } diff --git a/unitTests/test_types.cpp b/unitTests/test_types.cpp index 227a345e..ba77a11c 100644 --- a/unitTests/test_types.cpp +++ b/unitTests/test_types.cpp @@ -48,7 +48,8 @@ TEST(DataBuf, canBeConstructedFromExistingData) { TEST(DataBuf, tryingToAccessTooFarElementThrows) { const std::array data {'h', 'o', 'l', 'a'}; DataBuf instance(data.data(), data.size()); - ASSERT_THROW(instance.data(4), std::out_of_range); + 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 methods like DataBuf::read_uint32 and DataBuf::write_uint32. From 2b91b5daf9216119950b2eaab6735e88478e285e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Wed, 13 Apr 2022 13:51:56 +0200 Subject: [PATCH 04/13] More tests for DataBuf read/write functions --- src/types.cpp | 18 +++++++-------- unitTests/test_types.cpp | 47 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/src/types.cpp b/src/types.cpp index a3f691e9..1bffafd9 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -110,63 +110,63 @@ 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); } diff --git a/unitTests/test_types.cpp b/unitTests/test_types.cpp index ba77a11c..dba9e04f 100644 --- a/unitTests/test_types.cpp +++ b/unitTests/test_types.cpp @@ -40,18 +40,61 @@ TEST(DataBuf, allocatesDataWithNonEmptyConstructor) { } TEST(DataBuf, canBeConstructedFromExistingData) { - const std::array data {'h', 'o', 'l', 'a'}; + const std::array 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 data {'h', 'o', 'l', 'a'}; + const std::array 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 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 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); From 331924612e7cc91569194ceeede28e182369498b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Wed, 13 Apr 2022 14:14:01 +0200 Subject: [PATCH 05/13] Remove duplication by using templates --- src/types.cpp | 27 +++++++-------------------- unitTests/test_types.cpp | 30 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/types.cpp b/src/types.cpp index 1bffafd9..91d586ca 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -211,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 +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; @@ -232,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 - /// \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; + return fromStreamToRational(is, r); } uint16_t getUShort(const byte* buf, ByteOrder byteOrder) { diff --git a/unitTests/test_types.cpp b/unitTests/test_types.cpp index dba9e04f..2ef75cdf 100644 --- a/unitTests/test_types.cpp +++ b/unitTests/test_types.cpp @@ -143,3 +143,33 @@ 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(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); +} From 8da71e713337045633f049746b347897fe937280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Wed, 13 Apr 2022 14:22:20 +0200 Subject: [PATCH 06/13] Test parseUint32 and fix it when number is out of limits --- src/types.cpp | 1 + unitTests/test_types.cpp | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/types.cpp b/src/types.cpp index 91d586ca..8711e5e5 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -561,6 +561,7 @@ uint32_t parseUint32(const std::string& s, bool& ok) { if (ok && 0 <= x && x <= std::numeric_limits::max()) { return static_cast(x); } + ok = false; return 0; } diff --git a/unitTests/test_types.cpp b/unitTests/test_types.cpp index 2ef75cdf..fc901a92 100644 --- a/unitTests/test_types.cpp +++ b/unitTests/test_types.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -145,7 +146,7 @@ TEST(Rational, floatToRationalCast) { } TEST(Rational, toStream) { - Rational r = {1,2}; + Rational r = {1, 2}; std::stringstream str; str << r; ASSERT_EQ("1/2", str.str()); @@ -160,7 +161,7 @@ TEST(Rational, readRationalFromStream) { } TEST(URational, toStream) { - URational r = {1,2}; + URational r = {1, 2}; std::stringstream str; str << r; ASSERT_EQ("1/2", str.str()); @@ -173,3 +174,17 @@ TEST(URational, readRationalFromStream) { 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); +} From 1a3e93856bd4daf9b337340d9463054b7aae879a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Wed, 13 Apr 2022 15:20:28 +0200 Subject: [PATCH 07/13] More tests for rational convertions --- src/types.cpp | 10 +++---- unitTests/test_types.cpp | 56 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/types.cpp b/src/types.cpp index 8711e5e5..bea58c40 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -620,14 +620,12 @@ Rational floatToRationalCast(float f) { // Beware: primitive conversion algorithm int32_t den = 1000000; const auto d_as_long = static_cast(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(std::round(d * den)); const int32_t g = gcd(nom, den); diff --git a/unitTests/test_types.cpp b/unitTests/test_types.cpp index fc901a92..fdc855fc 100644 --- a/unitTests/test_types.cpp +++ b/unitTests/test_types.cpp @@ -160,6 +160,62 @@ TEST(Rational, readRationalFromStream) { 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; From d5742f449b1ea2411b75f9fdbdeb882df5560410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Wed, 13 Apr 2022 15:37:35 +0200 Subject: [PATCH 08/13] Move string helpers to utils --- src/pngimage.cpp | 7 +------ src/utils.cpp | 19 ++++++++++++++++++- src/utils.hpp | 12 ++++++++++-- src/xmpsidecar.cpp | 11 ++--------- unitTests/CMakeLists.txt | 1 + unitTests/test_utils.cpp | 23 +++++++++++++++++++++++ 6 files changed, 55 insertions(+), 18 deletions(-) create mode 100644 unitTests/test_utils.cpp diff --git a/src/pngimage.cpp b/src/pngimage.cpp index c801bdd7..795792ad 100644 --- a/src/pngimage.cpp +++ b/src/pngimage.cpp @@ -18,6 +18,7 @@ #include "pngimage.hpp" #include "tiffimage.hpp" #include "types.hpp" +#include "utils.hpp" #include #include @@ -171,12 +172,6 @@ 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)); } diff --git a/src/utils.cpp b/src/utils.cpp index e96e4c21..b3f9effd 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -1,7 +1,24 @@ #include "utils.hpp" +#include +#include +#include + namespace Exiv2 { -bool startsWith(const std::string_view& s, const std::string_view& start) { +bool startsWith(std::string_view s, std::string_view start) { return s.find(start) == 0; } + +std::string upper(const std::string& str) { + std::string result; + std::transform(str.begin(), str.end(), std::back_inserter(result), ::toupper); + return result; +} + +std::string lower(const std::string& a) { + std::string b = a; + std::transform(a.begin(), a.end(), b.begin(), ::tolower); + return b; +} + } // namespace Exiv2 diff --git a/src/utils.hpp b/src/utils.hpp index 1d90e3f7..a2bb5c15 100644 --- a/src/utils.hpp +++ b/src/utils.hpp @@ -1,10 +1,18 @@ #ifndef EXIV2_UTILS_HPP #define EXIV2_UTILS_HPP +#include #include namespace Exiv2 { -bool startsWith(const std::string_view& s, const std::string_view& start); -} +bool startsWith(std::string_view s, std::string_view start); + +/// @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 #endif // EXIV2_UTILS_HPP diff --git a/src/xmpsidecar.cpp b/src/xmpsidecar.cpp index 3b3698d6..53485c10 100644 --- a/src/xmpsidecar.cpp +++ b/src/xmpsidecar.cpp @@ -7,6 +7,7 @@ #include "error.hpp" #include "futils.hpp" #include "image.hpp" +#include "utils.hpp" #include "xmp_exiv2.hpp" #include @@ -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 lower(key).find(substr) != std::string::npos; } void XmpSidecar::writeMetadata() { diff --git a/unitTests/CMakeLists.txt b/unitTests/CMakeLists.txt index c6db700d..633847fc 100644 --- a/unitTests/CMakeLists.txt +++ b/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 $ ) diff --git a/unitTests/test_utils.cpp b/unitTests/test_utils.cpp new file mode 100644 index 00000000..2b39b005 --- /dev/null +++ b/unitTests/test_utils.cpp @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#include "utils.hpp" + +#include + +using namespace Exiv2; + +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")); +} From ee855c0e713b95921e313f6349e5c1a8d0e8b8b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Wed, 13 Apr 2022 15:44:17 +0200 Subject: [PATCH 09/13] Reduce amount of string transformations --- src/pngimage.cpp | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/pngimage.cpp b/src/pngimage.cpp index 795792ad..df4b1ec1 100644 --- a/src/pngimage.cpp +++ b/src/pngimage.cpp @@ -173,7 +173,7 @@ static bool tEXtToDataBuf(const byte* bytes, long length, DataBuf& 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) { @@ -189,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) { @@ -270,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) { From 74da8a394f77d733e3683d8a785f92a1390624f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Wed, 13 Apr 2022 16:03:33 +0200 Subject: [PATCH 10/13] Remove deprecated todo --- unitTests/test_types.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unitTests/test_types.cpp b/unitTests/test_types.cpp index fdc855fc..d4a8e034 100644 --- a/unitTests/test_types.cpp +++ b/unitTests/test_types.cpp @@ -36,7 +36,7 @@ TEST(DataBuf, defaultInstanceIsEmpty) { TEST(DataBuf, allocatesDataWithNonEmptyConstructor) { DataBuf instance(5); - ASSERT_NE(static_cast(nullptr), instance.c_data()); /// \todo use nullptr once we move to c++11 + ASSERT_NE(nullptr, instance.c_data()); ASSERT_EQ(5, instance.size()); } From cc79051b79b427d5d8ddeb3f6c2b35382e098132 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Wed, 13 Apr 2022 21:06:25 +0200 Subject: [PATCH 11/13] Move 1-liner to header file --- src/utils.cpp | 3 --- src/utils.hpp | 4 +++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/utils.cpp b/src/utils.cpp index b3f9effd..57e9fa9d 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -5,9 +5,6 @@ #include namespace Exiv2 { -bool startsWith(std::string_view s, std::string_view start) { - return s.find(start) == 0; -} std::string upper(const std::string& str) { std::string result; diff --git a/src/utils.hpp b/src/utils.hpp index a2bb5c15..a8f21c43 100644 --- a/src/utils.hpp +++ b/src/utils.hpp @@ -5,7 +5,9 @@ #include namespace Exiv2 { -bool startsWith(std::string_view s, 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); From d4ffcb9497b6bddc5842feeb69563dfb4ba65d17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Thu, 14 Apr 2022 08:05:27 +0200 Subject: [PATCH 12/13] utils are in Internal namespace --- src/epsimage.cpp | 2 +- src/utils.cpp | 4 ++-- src/utils.hpp | 5 +++-- src/xmpsidecar.cpp | 2 +- unitTests/test_utils.cpp | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/epsimage.cpp b/src/epsimage.cpp index 9db942f7..4c1d30d9 100644 --- a/src/epsimage.cpp +++ b/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"); diff --git a/src/utils.cpp b/src/utils.cpp index 57e9fa9d..111aff00 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -4,7 +4,7 @@ #include #include -namespace Exiv2 { +namespace Exiv2::Internal { std::string upper(const std::string& str) { std::string result; @@ -18,4 +18,4 @@ std::string lower(const std::string& a) { return b; } -} // namespace Exiv2 +} // namespace Exiv2::Internal diff --git a/src/utils.hpp b/src/utils.hpp index a8f21c43..1e16648d 100644 --- a/src/utils.hpp +++ b/src/utils.hpp @@ -4,7 +4,8 @@ #include #include -namespace Exiv2 { +namespace Exiv2::Internal { + constexpr bool startsWith(std::string_view s, std::string_view start) { return s.find(start) == 0; } @@ -15,6 +16,6 @@ std::string upper(const std::string& str); /// @brief Returns the lowercase version of \b str std::string lower(const std::string& str); -} // namespace Exiv2 +} // namespace Exiv2::Internal #endif // EXIV2_UTILS_HPP diff --git a/src/xmpsidecar.cpp b/src/xmpsidecar.cpp index 53485c10..ecb95027 100644 --- a/src/xmpsidecar.cpp +++ b/src/xmpsidecar.cpp @@ -84,7 +84,7 @@ void XmpSidecar::readMetadata() { } // XmpSidecar::readMetadata static bool matchi(const std::string& key, const char* substr) { - return lower(key).find(substr) != std::string::npos; + return Internal::lower(key).find(substr) != std::string::npos; } void XmpSidecar::writeMetadata() { diff --git a/unitTests/test_utils.cpp b/unitTests/test_utils.cpp index 2b39b005..f94638e3 100644 --- a/unitTests/test_utils.cpp +++ b/unitTests/test_utils.cpp @@ -4,7 +4,7 @@ #include -using namespace Exiv2; +using namespace Exiv2::Internal; TEST(stringUtils, startsWithReturnsTrue) { ASSERT_TRUE(startsWith("Exiv2 rocks", "Exiv2")); From 2b74cc885d4825aa3927fea9125b8250f3a51f94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Thu, 14 Apr 2022 13:22:45 +0200 Subject: [PATCH 13/13] Revert some boilerplate which is not needed --- src/bmffimage.cpp | 2 +- src/jpgimage.cpp | 4 ++-- src/rafimage.cpp | 4 +--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/bmffimage.cpp b/src/bmffimage.cpp index 977e1be8..a45076c0 100644 --- a/src/bmffimage.cpp +++ b/src/bmffimage.cpp @@ -620,7 +620,7 @@ void BmffImage::printStructure(std::ostream& out, Exiv2::PrintStructureOption op break; // do nothing case kpsIccProfile: { - out.write(iccProfile_.empty() ? nullptr : iccProfile_.c_str(), iccProfile_.size()); + out.write(iccProfile_.c_str(), iccProfile_.size()); } break; #ifdef EXV_HAVE_XMP_TOOLKIT diff --git a/src/jpgimage.cpp b/src/jpgimage.cpp index 57ce060c..010080aa 100644 --- a/src/jpgimage.cpp +++ b/src/jpgimage.cpp @@ -746,7 +746,7 @@ void JpegBase::doWriteMetadata(BasicIo& outIo) { bo = littleEndian; setByteOrder(bo); } - const byte* pExifData = rawExif.empty() ? nullptr : rawExif.c_data(); + const byte* pExifData = rawExif.c_data(); size_t exifSize = rawExif.size(); WriteMethod wm = ExifParser::encode(blob, pExifData, exifSize, bo, exifData_); if (wm == wmIntrusive) { @@ -843,7 +843,7 @@ 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.empty() ? nullptr : newPsData.c_data(); const byte* chunkEnd = newPsData.empty() ? nullptr : newPsData.c_data(newPsData.size() - 1); diff --git a/src/rafimage.cpp b/src/rafimage.cpp index 0b048c45..8d8c097d 100644 --- a/src/rafimage.cpp +++ b/src/rafimage.cpp @@ -265,9 +265,7 @@ void RafImage::readMetadata() { throw Error(ErrorCode::kerFailedToReadImageData); } - // io_->seek(0, BasicIo::beg); // rewind - - ByteOrder bo = TiffParser::decode(exifData_, iptcData_, xmpData_, buf.empty() ? nullptr : buf.c_data(), buf.size()); + ByteOrder bo = TiffParser::decode(exifData_, iptcData_, xmpData_, buf.c_data(), buf.size()); exifData_["Exif.Image2.JPEGInterchangeFormat"] = getULong(jpg_img_offset, bigEndian); exifData_["Exif.Image2.JPEGInterchangeFormatLength"] = getULong(jpg_img_length, bigEndian);