From 40fefd725bb953d3832a0c680f95628b9a3ae24a Mon Sep 17 00:00:00 2001 From: ptahmose Date: Fri, 15 Sep 2023 18:51:07 +0200 Subject: [PATCH 01/10] makeshift-fix for "preserving order of attachments in attachments-directory" --- Src/libCZI/CziAttachmentsDirectory.cpp | 46 ++++++-------------------- Src/libCZI/CziAttachmentsDirectory.h | 6 +--- Src/libCZI/CziWriter.cpp | 2 +- 3 files changed, 12 insertions(+), 42 deletions(-) diff --git a/Src/libCZI/CziAttachmentsDirectory.cpp b/Src/libCZI/CziAttachmentsDirectory.cpp index 04dc89c6..3228f618 100644 --- a/Src/libCZI/CziAttachmentsDirectory.cpp +++ b/Src/libCZI/CziAttachmentsDirectory.cpp @@ -4,6 +4,7 @@ #include "stdafx.h" #include "CziAttachmentsDirectory.h" +#include /*static*/bool CCziAttachmentsDirectoryBase::CompareForEquality_Id(const AttachmentEntry& a, const AttachmentEntry& b) { @@ -63,14 +64,19 @@ bool CCziAttachmentsDirectory::TryGetAttachment(int index, AttachmentEntry& entr bool CWriterCziAttachmentsDirectory::TryAddAttachment(const AttachmentEntry& entry) { - auto insert = this->attachments.insert(entry); - return insert.second; + if (std::find_if(this->attachments_.cbegin(), this->attachments_.cend(), [&entry](const AttachmentEntry& x) { return CompareForEquality_Id(x, entry); }) != this->attachments_.cend()) + { + return false; + } + + this->attachments_.push_back(entry); + return true; } bool CWriterCziAttachmentsDirectory::EnumEntries(const std::function& func) const { int index = 0; - for (auto it = this->attachments.cbegin(); it != this->attachments.cend(); ++it) + for (auto it = this->attachments_.cbegin(); it != this->attachments_.cend(); ++it) { if (!func(index++, *it)) { @@ -83,39 +89,7 @@ bool CWriterCziAttachmentsDirectory::EnumEntries(const std::functionattachments.size(); -} - -bool CWriterCziAttachmentsDirectory::AttachmentEntriesCompare::operator()(const AttachmentEntry& a, const AttachmentEntry& b) const -{ - // we shall return true if a is considered to go before b in the strict weak ordering the function defines - int r = memcmp(&a.ContentGuid, &b.ContentGuid, sizeof(GUID)); - if (r < 0) - { - return true; - } - else if (r > 0) - { - return false; - } - - r = memcmp(&a.ContentFileType, &b.ContentFileType, sizeof(a.ContentFileType)); - if (r < 0) - { - return true; - } - else if (r > 0) - { - return false; - } - - r = memcmp(&a.Name, &b.Name, sizeof(a.Name)); - if (r < 0) - { - return true; - } - - return false; + return (int)this->attachments_.size(); } //----------------------------------------------------------------------------- diff --git a/Src/libCZI/CziAttachmentsDirectory.h b/Src/libCZI/CziAttachmentsDirectory.h index 3b713c66..329a3c6b 100644 --- a/Src/libCZI/CziAttachmentsDirectory.h +++ b/Src/libCZI/CziAttachmentsDirectory.h @@ -50,11 +50,7 @@ class CWriterCziAttachmentsDirectory : public CCziAttachmentsDirectoryBase int GetAttachmentCount() const; bool EnumEntries(const std::function& func) const; private: - struct AttachmentEntriesCompare { - bool operator() (const AttachmentEntry& a, const AttachmentEntry& b) const; - }; - - std::set attachments; + std::vector attachments_; }; class CReaderWriterCziAttachmentsDirectory : public CCziAttachmentsDirectoryBase diff --git a/Src/libCZI/CziWriter.cpp b/Src/libCZI/CziWriter.cpp index 2c4813a9..883e8c67 100644 --- a/Src/libCZI/CziWriter.cpp +++ b/Src/libCZI/CziWriter.cpp @@ -1161,7 +1161,7 @@ void CCziWriter::WriteAttachmentDirectory() } } -std::tuple CCziWriter::WriteCurrentAttachmentsDirectory() +std::tuple CCziWriter::WriteCurrentAttachmentsDirectory() { CWriterUtils::AttachmentDirWriteInfo info; if (this->attachmentDirectorySegment.IsValid()) From 97b09d814f1738b634614045b57d4275f7475302 Mon Sep 17 00:00:00 2001 From: ptahmose Date: Sat, 16 Sep 2023 01:23:41 +0200 Subject: [PATCH 02/10] some cosmetic --- Src/libCZI/CziAttachmentsDirectory.cpp | 20 +++++++++++--------- Src/libCZI/CziAttachmentsDirectory.h | 7 +++++++ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Src/libCZI/CziAttachmentsDirectory.cpp b/Src/libCZI/CziAttachmentsDirectory.cpp index 3228f618..6a6956a6 100644 --- a/Src/libCZI/CziAttachmentsDirectory.cpp +++ b/Src/libCZI/CziAttachmentsDirectory.cpp @@ -8,13 +8,12 @@ /*static*/bool CCziAttachmentsDirectoryBase::CompareForEquality_Id(const AttachmentEntry& a, const AttachmentEntry& b) { - int r = memcmp(&a.ContentGuid, &b.ContentGuid, sizeof(GUID)); - if (r != 0) + if (!IsEqualGUID(a.ContentGuid, b.ContentGuid)) { return false; } - r = strncmp(a.Name, b.Name, sizeof(a.Name)); + int r = strncmp(a.Name, b.Name, sizeof(a.Name)); if (r != 0) { return false; @@ -64,7 +63,10 @@ bool CCziAttachmentsDirectory::TryGetAttachment(int index, AttachmentEntry& entr bool CWriterCziAttachmentsDirectory::TryAddAttachment(const AttachmentEntry& entry) { - if (std::find_if(this->attachments_.cbegin(), this->attachments_.cend(), [&entry](const AttachmentEntry& x) { return CompareForEquality_Id(x, entry); }) != this->attachments_.cend()) + if (std::find_if( + this->attachments_.cbegin(), + this->attachments_.cend(), + [&entry](const AttachmentEntry& x) { return CCziAttachmentsDirectoryBase::CompareForEquality_Id(x, entry); }) != this->attachments_.cend()) { return false; } @@ -174,12 +176,12 @@ bool CReaderWriterCziAttachmentsDirectory::TryRemoveAttachment(int key, Attachme bool CReaderWriterCziAttachmentsDirectory::TryAddAttachment(const AttachmentEntry& entry, int* key) { - for (auto it = this->attchmnts.cbegin(); it != this->attchmnts.cend(); ++it) + if (std::find_if( + this->attchmnts.cbegin(), + this->attchmnts.cend(), + [&entry](const auto& x) { return CCziAttachmentsDirectoryBase::CompareForEquality_Id(x.second, entry); }) != this->attchmnts.cend()) { - if (CCziAttachmentsDirectoryBase::CompareForEquality_Id(it->second, entry)) - { - return false; - } + return false; } this->AddAttachment(entry, key); diff --git a/Src/libCZI/CziAttachmentsDirectory.h b/Src/libCZI/CziAttachmentsDirectory.h index 329a3c6b..52955cec 100644 --- a/Src/libCZI/CziAttachmentsDirectory.h +++ b/Src/libCZI/CziAttachmentsDirectory.h @@ -24,6 +24,13 @@ class CCziAttachmentsDirectoryBase std::int64_t allocatedSize; }; + /// Compare the properties which make up the identity of an AttachmentEntry, + /// i.e. the GUID, the name and the content file type, for equality. + /// + /// \param a The first AttachmentEntry to process. + /// \param b The second AttachmentEntry to process. + /// + /// \returns True if the identifier-properties compare as equal; false otherwise. static bool CompareForEquality_Id(const AttachmentEntry& a, const AttachmentEntry& b); }; From cb4245a18dd007a28fff76cb15e31108a3aae6d1 Mon Sep 17 00:00:00 2001 From: ptahmose Date: Thu, 12 Oct 2023 15:57:52 +0200 Subject: [PATCH 03/10] bump version --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 96073bb3..02969952 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.15) cmake_policy(SET CMP0091 NEW) # enable new "MSVC runtime library selection" (https://cmake.org/cmake/help/latest/variable/CMAKE_MSVC_RUNTIME_LIBRARY.html) project(libCZI - VERSION 0.52.0 + VERSION 0.52.1 HOMEPAGE_URL "https://github.com/ZEISS/libczi" DESCRIPTION "libCZI is an Open Source Cross-Platform C++ library to read and write CZI") From 018fcef20ee1b4a4ec0e5c4bd72fb45531357d2e Mon Sep 17 00:00:00 2001 From: ptahmose Date: Thu, 12 Oct 2023 16:03:37 +0200 Subject: [PATCH 04/10] disable trivy from megalinter --- .mega-linter.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.mega-linter.yml b/.mega-linter.yml index d5a68f58..e837e2b4 100644 --- a/.mega-linter.yml +++ b/.mega-linter.yml @@ -10,6 +10,8 @@ DISABLE: - SPELL # Comment to enable checks of spelling mistakes DISABLE_ERRORS_LINTERS: - MARKDOWN_MARKDOWN_LINK_CHECK # Make non-blocking due to network timeouts etc. +DISABLE_LINTERS: + - REPOSITORY_TRIVY # this linter seems currently broken, so we disable it here for now C_CPPLINT_ARGUMENTS: --verbose=2 CPP_CPPLINT_ARGUMENTS: --verbose=2 SHOW_ELAPSED_TIME: true From 579972a67aaa2a215b1ec413998e44e2b65920b8 Mon Sep 17 00:00:00 2001 From: ptahmose Date: Thu, 12 Oct 2023 16:30:57 +0200 Subject: [PATCH 05/10] no auto... --- Src/libCZI/CziAttachmentsDirectory.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Src/libCZI/CziAttachmentsDirectory.cpp b/Src/libCZI/CziAttachmentsDirectory.cpp index 6a6956a6..037bb922 100644 --- a/Src/libCZI/CziAttachmentsDirectory.cpp +++ b/Src/libCZI/CziAttachmentsDirectory.cpp @@ -64,9 +64,9 @@ bool CCziAttachmentsDirectory::TryGetAttachment(int index, AttachmentEntry& entr bool CWriterCziAttachmentsDirectory::TryAddAttachment(const AttachmentEntry& entry) { if (std::find_if( - this->attachments_.cbegin(), - this->attachments_.cend(), - [&entry](const AttachmentEntry& x) { return CCziAttachmentsDirectoryBase::CompareForEquality_Id(x, entry); }) != this->attachments_.cend()) + this->attachments_.cbegin(), + this->attachments_.cend(), + [&entry](const AttachmentEntry& x)->bool { return CCziAttachmentsDirectoryBase::CompareForEquality_Id(x, entry); }) != this->attachments_.cend()) { return false; } @@ -177,9 +177,9 @@ bool CReaderWriterCziAttachmentsDirectory::TryRemoveAttachment(int key, Attachme bool CReaderWriterCziAttachmentsDirectory::TryAddAttachment(const AttachmentEntry& entry, int* key) { if (std::find_if( - this->attchmnts.cbegin(), - this->attchmnts.cend(), - [&entry](const auto& x) { return CCziAttachmentsDirectoryBase::CompareForEquality_Id(x.second, entry); }) != this->attchmnts.cend()) + this->attchmnts.cbegin(), + this->attchmnts.cend(), + [&entry](const std::pair& x)->bool { return CCziAttachmentsDirectoryBase::CompareForEquality_Id(x.second, entry); }) != this->attchmnts.cend()) { return false; } From 12c34674484e808045a0350a44dedfa48407a02d Mon Sep 17 00:00:00 2001 From: ptahmose Date: Mon, 16 Oct 2023 10:58:39 +0200 Subject: [PATCH 06/10] undo removal of "trivy" --- .mega-linter.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.mega-linter.yml b/.mega-linter.yml index e837e2b4..d5a68f58 100644 --- a/.mega-linter.yml +++ b/.mega-linter.yml @@ -10,8 +10,6 @@ DISABLE: - SPELL # Comment to enable checks of spelling mistakes DISABLE_ERRORS_LINTERS: - MARKDOWN_MARKDOWN_LINK_CHECK # Make non-blocking due to network timeouts etc. -DISABLE_LINTERS: - - REPOSITORY_TRIVY # this linter seems currently broken, so we disable it here for now C_CPPLINT_ARGUMENTS: --verbose=2 CPP_CPPLINT_ARGUMENTS: --verbose=2 SHOW_ELAPSED_TIME: true From 6f57b67f3d5f096fa5b62faf9439e6fd07925884 Mon Sep 17 00:00:00 2001 From: ptahmose Date: Thu, 19 Oct 2023 14:43:19 +0200 Subject: [PATCH 07/10] disable "TRIVY" linter (seems to be broken) --- .mega-linter.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.mega-linter.yml b/.mega-linter.yml index d5a68f58..e837e2b4 100644 --- a/.mega-linter.yml +++ b/.mega-linter.yml @@ -10,6 +10,8 @@ DISABLE: - SPELL # Comment to enable checks of spelling mistakes DISABLE_ERRORS_LINTERS: - MARKDOWN_MARKDOWN_LINK_CHECK # Make non-blocking due to network timeouts etc. +DISABLE_LINTERS: + - REPOSITORY_TRIVY # this linter seems currently broken, so we disable it here for now C_CPPLINT_ARGUMENTS: --verbose=2 CPP_CPPLINT_ARGUMENTS: --verbose=2 SHOW_ELAPSED_TIME: true From 1e65833b96093d743934553290623f17c60a31de Mon Sep 17 00:00:00 2001 From: ptahmose Date: Thu, 19 Oct 2023 15:53:05 +0200 Subject: [PATCH 08/10] wip --- Src/libCZI_UnitTests/test_writer.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Src/libCZI_UnitTests/test_writer.cpp b/Src/libCZI_UnitTests/test_writer.cpp index 5c12e441..7b5a76c2 100644 --- a/Src/libCZI_UnitTests/test_writer.cpp +++ b/Src/libCZI_UnitTests/test_writer.cpp @@ -2121,3 +2121,24 @@ TEST(CziWriter, WriteReadCompressedZStd1ImageBrg48LowPacking) constexpr PixelType pixelType = PixelType::Bgr48; _testWriteReadCompressedImageZStd1LowPacking(61, 61, pixelType, 2, true); } + +TEST(CziWriter, TryAddingDuplicateAttachmentAndExpectError) +{ + auto writer = CreateCZIWriter(); + auto outStream = make_shared(0); + + // GUID_NULL here means that a new Guid is created + const auto czi_writer_info = std::make_shared(GUID{ 0, 0, 0, {0, 0, 0, 0, 0, 0, 0, 0} }); + + writer->Create(outStream, czi_writer_info); + + uint8_t data[] = { 1,2,3 }; + + AddAttachmentInfo add_attachment_info; + add_attachment_info.contentGuid = GUID{ 1, 2, 3, {4, 5, 6, 7, 8, 9, 10, 11} }; + add_attachment_info.SetContentFileType("ABC"); + add_attachment_info.SetName("Test"); + add_attachment_info.ptrData = data; + add_attachment_info.dataSize = sizeof(data); + writer->SyncAddAttachment(add_attachment_info); +} From 11210963912d0e4ba4df49907cf0395404c9bbc8 Mon Sep 17 00:00:00 2001 From: ptahmose Date: Thu, 19 Oct 2023 23:52:49 +0200 Subject: [PATCH 09/10] add unittest (giving code-coverage for the requested line) --- Src/libCZI_UnitTests/test_readerwriter.cpp | 35 ++++++++++++++++++++++ Src/libCZI_UnitTests/test_writer.cpp | 14 +++++---- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/Src/libCZI_UnitTests/test_readerwriter.cpp b/Src/libCZI_UnitTests/test_readerwriter.cpp index 6dfe0749..1790eeea 100644 --- a/Src/libCZI_UnitTests/test_readerwriter.cpp +++ b/Src/libCZI_UnitTests/test_readerwriter.cpp @@ -1352,3 +1352,38 @@ INSTANTIATE_TEST_SUITE_P( SubBlockPyramidType::None, SubBlockPyramidType::SingleSubBlock, SubBlockPyramidType::MultiSubBlock)); + +TEST(CziReaderWriter, TryAddingDuplicateAttachmentToCziReaderWriterAndExpectError) +{ + const auto in_out_stream = make_shared(0); + const auto reader_writer = CreateCZIReaderWriter(); + reader_writer->Create(in_out_stream); + + constexpr uint8_t data[] = { 1,2,3 }; + + AddAttachmentInfo add_attachment_info; + add_attachment_info.contentGuid = GUID{ 1, 2, 3, {4, 5, 6, 7, 8, 9, 10, 11} }; + add_attachment_info.SetContentFileType("ABC"); + add_attachment_info.SetName("Test"); + add_attachment_info.ptrData = data; + add_attachment_info.dataSize = sizeof(data); + reader_writer->SyncAddAttachment(add_attachment_info); + + // now, try to add it a second time + EXPECT_THROW(reader_writer->SyncAddAttachment(add_attachment_info), LibCZIException); + + // a duplicate attachment means - same Guid, same name, same content-type, + // so now let's try to add it with a different name (which should work) + add_attachment_info.SetName("Test2"); + reader_writer->SyncAddAttachment(add_attachment_info); + + // now with a different content-type (which should also work) + add_attachment_info.SetName("Test"); + add_attachment_info.SetContentFileType("ABCD"); + reader_writer->SyncAddAttachment(add_attachment_info); + + // and finally with a different GUID + add_attachment_info.SetContentFileType("ABC"); + add_attachment_info.contentGuid = GUID{ 111, 2, 3, {4, 5, 6, 7, 8, 9, 10, 11} }; + reader_writer->SyncAddAttachment(add_attachment_info); +} diff --git a/Src/libCZI_UnitTests/test_writer.cpp b/Src/libCZI_UnitTests/test_writer.cpp index 7b5a76c2..71750a60 100644 --- a/Src/libCZI_UnitTests/test_writer.cpp +++ b/Src/libCZI_UnitTests/test_writer.cpp @@ -2122,17 +2122,16 @@ TEST(CziWriter, WriteReadCompressedZStd1ImageBrg48LowPacking) _testWriteReadCompressedImageZStd1LowPacking(61, 61, pixelType, 2, true); } -TEST(CziWriter, TryAddingDuplicateAttachmentAndExpectError) +TEST(CziWriter, TryAddingDuplicateAttachmentToCziWriterAndExpectError) { - auto writer = CreateCZIWriter(); - auto outStream = make_shared(0); + const auto writer = CreateCZIWriter(); + const auto output_stream = make_shared(0); - // GUID_NULL here means that a new Guid is created const auto czi_writer_info = std::make_shared(GUID{ 0, 0, 0, {0, 0, 0, 0, 0, 0, 0, 0} }); - writer->Create(outStream, czi_writer_info); + writer->Create(output_stream, czi_writer_info); - uint8_t data[] = { 1,2,3 }; + constexpr uint8_t data[] = { 1,2,3 }; AddAttachmentInfo add_attachment_info; add_attachment_info.contentGuid = GUID{ 1, 2, 3, {4, 5, 6, 7, 8, 9, 10, 11} }; @@ -2141,4 +2140,7 @@ TEST(CziWriter, TryAddingDuplicateAttachmentAndExpectError) add_attachment_info.ptrData = data; add_attachment_info.dataSize = sizeof(data); writer->SyncAddAttachment(add_attachment_info); + + // now, try to add it a second time + EXPECT_THROW(writer->SyncAddAttachment(add_attachment_info), LibCZIException); } From 861ad2addf30f2f1bd338dcc6b7b69b0be0cad1d Mon Sep 17 00:00:00 2001 From: ptahmose Date: Fri, 20 Oct 2023 00:13:18 +0200 Subject: [PATCH 10/10] fix --- Src/libCZI/CziAttachmentsDirectory.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Src/libCZI/CziAttachmentsDirectory.cpp b/Src/libCZI/CziAttachmentsDirectory.cpp index 3d8f5872..fc8c55fc 100644 --- a/Src/libCZI/CziAttachmentsDirectory.cpp +++ b/Src/libCZI/CziAttachmentsDirectory.cpp @@ -4,6 +4,7 @@ #include "CziAttachmentsDirectory.h" #include +#include /*static*/bool CCziAttachmentsDirectoryBase::CompareForEquality_Id(const AttachmentEntry& a, const AttachmentEntry& b) {