Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jbl/1020721 preserve order in attachmentsdirectory #68

Merged
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
64 changes: 20 additions & 44 deletions Src/libCZI/CziAttachmentsDirectory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@

#include "stdafx.h"
#include "CziAttachmentsDirectory.h"
#include <algorithm>

/*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;
Expand Down Expand Up @@ -63,14 +63,22 @@

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)->bool { return CCziAttachmentsDirectoryBase::CompareForEquality_Id(x, entry); }) != this->attachments_.cend())
{
return false;
}

this->attachments_.push_back(entry);
return true;
}

bool CWriterCziAttachmentsDirectory::EnumEntries(const std::function<bool(int index, const AttachmentEntry&)>& 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))
{
Expand All @@ -83,39 +91,7 @@

int CWriterCziAttachmentsDirectory::GetAttachmentCount() const
{
return (int)this->attachments.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();
}

//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -200,12 +176,12 @@

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 std::pair<int, AttachmentEntry>& x)->bool { return CCziAttachmentsDirectoryBase::CompareForEquality_Id(x.second, entry); }) != this->attchmnts.cend())
{
if (CCziAttachmentsDirectoryBase::CompareForEquality_Id(it->second, entry))
{
return false;
}
return false;

Check warning on line 184 in Src/libCZI/CziAttachmentsDirectory.cpp

View check run for this annotation

Codecov / codecov/patch

Src/libCZI/CziAttachmentsDirectory.cpp#L184

Added line #L184 was not covered by tests
chriwiz marked this conversation as resolved.
Show resolved Hide resolved
}

this->AddAttachment(entry, key);
Expand Down
13 changes: 8 additions & 5 deletions Src/libCZI/CziAttachmentsDirectory.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

Expand All @@ -50,11 +57,7 @@ class CWriterCziAttachmentsDirectory : public CCziAttachmentsDirectoryBase
int GetAttachmentCount() const;
bool EnumEntries(const std::function<bool(int index, const AttachmentEntry&)>& func) const;
private:
struct AttachmentEntriesCompare {
bool operator() (const AttachmentEntry& a, const AttachmentEntry& b) const;
};

std::set<AttachmentEntry, AttachmentEntriesCompare> attachments;
std::vector<AttachmentEntry> attachments_;
};

class CReaderWriterCziAttachmentsDirectory : public CCziAttachmentsDirectoryBase
Expand Down
2 changes: 1 addition & 1 deletion Src/libCZI/CziWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,7 @@ void CCziWriter::WriteAttachmentDirectory()
}
}

std::tuple<std::uint64_t, std::uint64_t> CCziWriter::WriteCurrentAttachmentsDirectory()
std::tuple<std::uint64_t, std::uint64_t> CCziWriter::WriteCurrentAttachmentsDirectory()
{
CWriterUtils::AttachmentDirWriteInfo info;
if (this->attachmentDirectorySegment.IsValid())
Expand Down