From c7853df78fa18c23cb1fcfcc964e33df24f7c0c7 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 30 Aug 2023 16:30:34 +0200 Subject: [PATCH 01/12] Move to c++17. All our compilers should handle c++17. Let's move on. --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 8a4a0f0d5..c223c408a 100644 --- a/meson.build +++ b/meson.build @@ -1,7 +1,7 @@ project('libzim', ['c', 'cpp'], version : '8.2.1', license : 'GPL2', - default_options : ['c_std=c11', 'cpp_std=c++11']) + default_options : ['c_std=c11', 'cpp_std=c++17']) if build_machine.system() != 'windows' add_project_arguments('-D_LARGEFILE64_SOURCE=1', '-D_FILE_OFFSET_BITS=64', language: 'cpp') From 719a61c9bb8593bd16a670dd9482818591b82a1d Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 30 Aug 2023 16:33:03 +0200 Subject: [PATCH 02/12] Add missing include. `uint32_t` is defined in `cstdint`. --- test/random.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/random.cpp b/test/random.cpp index 1a0cb2c49..cedb24882 100644 --- a/test/random.cpp +++ b/test/random.cpp @@ -18,6 +18,7 @@ */ #include "gtest/gtest.h" +#include namespace zim { uint32_t randomNumber(uint32_t max); From 56b1d9e7480e4055eda8f28f0046c4bec9bdfbcb Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 30 Aug 2023 17:22:12 +0200 Subject: [PATCH 03/12] Do not inherit from `std::iterator`. `std::iterator` is deprecated in c++17. --- include/zim/archive.h | 24 +++++++++++++++++++++++- include/zim/search_iterator.h | 26 +++++++++++++++++++++++++- include/zim/suggestion_iterator.h | 27 ++++++++++++++++++++++++++- 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/include/zim/archive.h b/include/zim/archive.h index dd98d9eed..7539b96c6 100644 --- a/include/zim/archive.h +++ b/include/zim/archive.h @@ -539,11 +539,33 @@ namespace zim * from race-condition. It is not threadsafe. * * An `EntryRange` can't be modified and is consequently threadsafe. + * + * Be aware that the referenced/pointed Entry is generated and stored + * in the iterator itself. + * Once the iterator is destructed or incremented/decremented, you must NOT + * use the Entry. */ template - class LIBZIM_API Archive::iterator : public std::iterator + class LIBZIM_API Archive::iterator { public: + /* SuggestionIterator is conceptually a bidirectional iterator. + * But std *LegayBidirectionalIterator* is also a *LegacyForwardIterator* and + * it would impose us that : + * > Given a and b, dereferenceable iterators of type It: + * > If a and b compare equal (a == b is contextually convertible to true) + * > then either they are both non-dereferenceable or *a and *b are references bound to the same object. + * and + * > the LegacyForwardIterator requirements requires dereference to return a reference. + * Which cannot be as we create the entry on demand. + * + * So we are stick with declaring ourselves at `input_iterator`. + */ + using iterator_category = std::input_iterator_tag; + using value_type = Entry; + using pointer = Entry*; + using reference = Entry&; + explicit iterator(const std::shared_ptr file, entry_index_type idx) : m_file(file), m_idx(idx), diff --git a/include/zim/search_iterator.h b/include/zim/search_iterator.h index 35ce206ca..9cdaf1d86 100644 --- a/include/zim/search_iterator.h +++ b/include/zim/search_iterator.h @@ -32,10 +32,34 @@ namespace zim { class SearchResultSet; -class LIBZIM_API SearchIterator : public std::iterator +/** + * A interator on search result (an Entry) + * + * Be aware that the referenced/pointed Entry is generated and stored + * in the iterator itself. + * Once the iterator is destructed or incremented/decremented, you must NOT + * use the Entry. + */ +class LIBZIM_API SearchIterator { friend class zim::SearchResultSet; public: + /* SuggestionIterator is conceptually a bidirectional iterator. + * But std *LegayBidirectionalIterator* is also a *LegacyForwardIterator* and + * it would impose us that : + * > Given a and b, dereferenceable iterators of type It: + * > If a and b compare equal (a == b is contextually convertible to true) + * > then either they are both non-dereferenceable or *a and *b are references bound to the same object. + * and + * > the LegacyForwardIterator requirements requires dereference to return a reference. + * Which cannot be as we create the entry on demand. + * + * So we are stick with declaring ourselves at `input_iterator`. + */ + using iterator_category = std::input_iterator_tag; + using value_type = Entry; + using pointer = Entry*; + using reference = Entry&; SearchIterator(); SearchIterator(const SearchIterator& it); SearchIterator& operator=(const SearchIterator& it); diff --git a/include/zim/suggestion_iterator.h b/include/zim/suggestion_iterator.h index 30ac9563a..9af2dd6b5 100644 --- a/include/zim/suggestion_iterator.h +++ b/include/zim/suggestion_iterator.h @@ -31,11 +31,36 @@ class SuggestionResultSet; class SuggestionItem; class SearchIterator; -class LIBZIM_API SuggestionIterator : public std::iterator +/** + * A interator on suggestion. + * + * Be aware that the referenced/pointed SuggestionItem is generated and stored + * in the iterator itself. + * Once the iterator is destructed or incremented/decremented, you must NOT + * use the SuggestionItem. + */ +class LIBZIM_API SuggestionIterator { typedef Archive::iterator RangeIterator; friend class SuggestionResultSet; public: + /* SuggestionIterator is conceptually a bidirectional iterator. + * But std *LegayBidirectionalIterator* is also a *LegacyForwardIterator* and + * it would impose us that : + * > Given a and b, dereferenceable iterators of type It: + * > If a and b compare equal (a == b is contextually convertible to true) + * > then either they are both non-dereferenceable or *a and *b are references bound to the same object. + * and + * > the LegacyForwardIterator requirements requires dereference to return a reference. + * Which cannot be as we create the entry on demand. + * + * So we are stick with declaring ourselves at `input_iterator`. + */ + using iterator_category = std::input_iterator_tag; + using value_type = SuggestionItem; + using pointer = SuggestionItem*; + using reference = SuggestionItem&; + SuggestionIterator() = delete; SuggestionIterator(const SuggestionIterator& it); SuggestionIterator& operator=(const SuggestionIterator& it); From 0d9ec3f7a1a148139b390e1eebd75c331ea10729 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 30 Aug 2023 17:57:07 +0200 Subject: [PATCH 04/12] Add `werror=true` --- meson.build | 2 +- src/fileimpl.cpp | 12 ------------ src/writer/dirent.cpp | 2 +- 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/meson.build b/meson.build index c223c408a..ca0fe5b8f 100644 --- a/meson.build +++ b/meson.build @@ -1,7 +1,7 @@ project('libzim', ['c', 'cpp'], version : '8.2.1', license : 'GPL2', - default_options : ['c_std=c11', 'cpp_std=c++17']) + default_options : ['c_std=c11', 'cpp_std=c++17', 'werror=true']) if build_machine.system() != 'windows' add_project_arguments('-D_LARGEFILE64_SOURCE=1', '-D_FILE_OFFSET_BITS=64', language: 'cpp') diff --git a/src/fileimpl.cpp b/src/fileimpl.cpp index f43a039e0..152f98aa0 100644 --- a/src/fileimpl.cpp +++ b/src/fileimpl.cpp @@ -381,18 +381,6 @@ class Grouping return { false, entry_index_t(0) }; } - static inline int direntCompareTitle(char ns, const std::string& title, const Dirent& dirent) - { - auto direntNs = dirent.getNamespace(); - if (ns < direntNs) { - return -1; - } - if (ns > direntNs) { - return 1; - } - return title.compare(dirent.getTitle()); - } - FileImpl::FindxTitleResult FileImpl::findxByTitle(char ns, const std::string& title) { return m_byTitleDirentLookup->find(ns, title); diff --git a/src/writer/dirent.cpp b/src/writer/dirent.cpp index f5df2ecc2..8b9990ec2 100644 --- a/src/writer/dirent.cpp +++ b/src/writer/dirent.cpp @@ -65,7 +65,7 @@ Dirent::Dirent(NS ns, const std::string& path, const std::string& title, NS targ : pathTitle(path, title), mimeType(redirectMimeType), idx(0), - info(std::move(DirentInfo::Redirect(targetNs, targetPath))), + info(DirentInfo::Redirect(targetNs, targetPath)), offset(0), _ns(static_cast(ns)), removed(false), From d6dcf113bbfce89c3f45a1eb7b3b68e6c4158c83 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 30 Aug 2023 18:04:46 +0200 Subject: [PATCH 05/12] Compare unsigned with unsigned. --- test/archive.cpp | 16 +++++----- test/bufferstreamer.cpp | 2 +- test/counterParsing.cpp | 2 +- test/creator.cpp | 2 +- test/defaultIndexdata.cpp | 12 ++++---- test/dirent.cpp | 6 ++-- test/dirent_lookup.cpp | 58 +++++++++++++++++------------------ test/find.cpp | 64 +++++++++++++++++++-------------------- test/lrucache.cpp | 12 ++++---- test/random.cpp | 4 +-- test/rawstreamreader.cpp | 4 +-- test/search.cpp | 10 +++--- test/tinyString.cpp | 20 ++++++------ test/tooltesting.cpp | 26 ++++++++-------- 14 files changed, 119 insertions(+), 119 deletions(-) diff --git a/test/archive.cpp b/test/archive.cpp index f543926ec..fb172834c 100644 --- a/test/archive.cpp +++ b/test/archive.cpp @@ -171,15 +171,15 @@ TEST(ZimArchive, openCreatedArchive) zim::Archive archive(tempPath); #if !defined(ENABLE_XAPIAN) // 2*listingIndex + M/Counter + M/Title + mainpage + 2*Illustration + 2*Item + redirection -#define ALL_ENTRY_COUNT 10 +#define ALL_ENTRY_COUNT 10U #else // same as above + 2 xapian indexes. -#define ALL_ENTRY_COUNT 12 +#define ALL_ENTRY_COUNT 12U #endif ASSERT_EQ(archive.getAllEntryCount(), ALL_ENTRY_COUNT); #undef ALL_ENTRY_COUNT - ASSERT_EQ(archive.getEntryCount(), 3); - ASSERT_EQ(archive.getArticleCount(), 1); + ASSERT_EQ(archive.getEntryCount(), 3U); + ASSERT_EQ(archive.getArticleCount(), 1U); ASSERT_EQ(archive.getUuid(), uuid); ASSERT_EQ(archive.getMetadataKeys(), std::vector({"Counter", "Illustration_48x48@1", "Illustration_96x96@1", "Title"})); ASSERT_EQ(archive.getIllustrationSizes(), std::set({48, 96})); @@ -492,7 +492,7 @@ void checkEquivalence(const zim::Archive& archive1, const zim::Archive& archive2 const zim::Entry mainEntry = archive1.getMainEntry(); ASSERT_EQ(mainEntry.getTitle(), archive2.getMainEntry().getTitle()); - ASSERT_NE(0, archive1.getEntryCount()); // ==> below loop is not a noop + ASSERT_NE(0U, archive1.getEntryCount()); // ==> below loop is not a noop { auto range1 = archive1.iterEfficient(); auto range2 = archive2.iterEfficient(); @@ -642,7 +642,7 @@ TEST(ZimArchive, getDirectAccessInformation) } } } - ASSERT_NE(0, checkedItemCount); + ASSERT_NE(0U, checkedItemCount); } } @@ -664,7 +664,7 @@ TEST(ZimArchive, getDirectAccessInformationInAnArchiveOpenedByFD) } } } - ASSERT_NE(0, checkedItemCount); + ASSERT_NE(0U, checkedItemCount); } } @@ -690,7 +690,7 @@ TEST(ZimArchive, getDirectAccessInformationFromEmbeddedArchive) } } } - ASSERT_NE(0, checkedItemCount); + ASSERT_NE(0U, checkedItemCount); } } #endif // not _WIN32 diff --git a/test/bufferstreamer.cpp b/test/bufferstreamer.cpp index d432c7a5f..4c7a9f8a1 100644 --- a/test/bufferstreamer.cpp +++ b/test/bufferstreamer.cpp @@ -41,7 +41,7 @@ TEST(BufferStreamer, shouldJustWork) auto buffer = Buffer::makeBuffer(data, zsize_t(sizeof(data))); zim::BufferStreamer bds(buffer, zsize_t(sizeof(data))); - ASSERT_EQ(1234, bds.read()); + ASSERT_EQ(1234U, bds.read()); ASSERT_EQ(data + 4, bds.current()); const auto blob1 = std::string(bds.current(), 4); diff --git a/test/counterParsing.cpp b/test/counterParsing.cpp index 180b77290..bc9cc9394 100644 --- a/test/counterParsing.cpp +++ b/test/counterParsing.cpp @@ -137,7 +137,7 @@ TEST(ParseCounterTest, wrongType) } #define CHECK(TEST, EXPECTED) \ -{ auto count = countMimeType(counterStr, [](const std::string& s) { return TEST;}); ASSERT_EQ(count, EXPECTED); } +{ auto count = countMimeType(counterStr, [](const std::string& s) { return TEST;}); ASSERT_EQ(count, (unsigned)EXPECTED); } TEST(ParseCounterTest, countMimeType) { { diff --git a/test/creator.cpp b/test/creator.cpp index c43270c72..aacebedcd 100644 --- a/test/creator.cpp +++ b/test/creator.cpp @@ -120,7 +120,7 @@ TEST(ZimCreator, createEmptyZim) Fileheader header; header.read(*reader); ASSERT_FALSE(header.hasMainPage()); - ASSERT_EQ(header.getArticleCount(), 2); // counter + titleListIndexesv0 + ASSERT_EQ(header.getArticleCount(), 2u); // counter + titleListIndexesv0 //Read the only one item existing. auto urlPtrReader = reader->sub_reader(offset_t(header.getUrlPtrPos()), zsize_t(sizeof(offset_t)*header.getArticleCount())); diff --git a/test/defaultIndexdata.cpp b/test/defaultIndexdata.cpp index d24ec8302..9956a6b07 100644 --- a/test/defaultIndexdata.cpp +++ b/test/defaultIndexdata.cpp @@ -38,7 +38,7 @@ namespace { ASSERT_EQ(indexData->getTitle(), "a title"); ASSERT_EQ(indexData->getContent(), ""); ASSERT_EQ(indexData->getKeywords(), ""); - ASSERT_EQ(indexData->getWordCount(), 0); + ASSERT_EQ(indexData->getWordCount(), 0U); ASSERT_EQ(indexData->getGeoPosition(), std::make_tuple(false, 0, 0)); } @@ -49,7 +49,7 @@ namespace { ASSERT_EQ(indexData->getTitle(), "a title"); ASSERT_EQ(indexData->getContent(), "some bold words"); ASSERT_EQ(indexData->getKeywords(), ""); - ASSERT_EQ(indexData->getWordCount(), 3); + ASSERT_EQ(indexData->getWordCount(), 3U); ASSERT_EQ(indexData->getGeoPosition(), std::make_tuple(false, 0, 0)); } @@ -60,7 +60,7 @@ namespace { ASSERT_EQ(indexData->getTitle(), "a title"); ASSERT_EQ(indexData->getContent(), ""); ASSERT_EQ(indexData->getKeywords(), ""); - ASSERT_EQ(indexData->getWordCount(), 0); + ASSERT_EQ(indexData->getWordCount(), 0U); ASSERT_EQ(indexData->getGeoPosition(), std::make_tuple(false, 0, 0)); } @@ -71,7 +71,7 @@ namespace { ASSERT_EQ(indexData->getTitle(), "a title"); ASSERT_EQ(indexData->getContent(), ""); ASSERT_EQ(indexData->getKeywords(), ""); - ASSERT_EQ(indexData->getWordCount(), 0); + ASSERT_EQ(indexData->getWordCount(), 0U); ASSERT_EQ(indexData->getGeoPosition(), std::make_tuple(false, 0, 0)); } @@ -82,7 +82,7 @@ namespace { ASSERT_EQ(indexData->getTitle(), "a title"); ASSERT_EQ(indexData->getContent(), "noindexsome bold words"); ASSERT_EQ(indexData->getKeywords(), ""); - ASSERT_EQ(indexData->getWordCount(), 3); + ASSERT_EQ(indexData->getWordCount(), 3U); ASSERT_EQ(indexData->getGeoPosition(), std::make_tuple(false, 0, 0)); } @@ -93,7 +93,7 @@ namespace { ASSERT_EQ(indexData->getTitle(), "a title"); ASSERT_EQ(indexData->getContent(), "some bold words"); ASSERT_EQ(indexData->getKeywords(), "some keyword important"); - ASSERT_EQ(indexData->getWordCount(), 3); + ASSERT_EQ(indexData->getWordCount(), 3U); auto geoPos = indexData->getGeoPosition(); ASSERT_TRUE(std::get<0>(geoPos)); ASSERT_TRUE(std::abs(std::get<1>(geoPos)-45.005) < 0.00001); diff --git a/test/dirent.cpp b/test/dirent.cpp index 5723abd4e..47479956d 100644 --- a/test/dirent.cpp +++ b/test/dirent.cpp @@ -67,16 +67,16 @@ size_t writenDirentSize(const zim::writer::Dirent& dirent) TEST(DirentTest, size) { #ifdef _WIN32 - ASSERT_EQ(sizeof(zim::writer::Dirent), 72); + ASSERT_EQ(sizeof(zim::writer::Dirent), 72U); #else // Dirent's size is important for us as we are creating huge zim files on linux // and we need to store a lot of dirents. // Be sure that dirent's size is not increased by any change. #if ENV32BIT // On 32 bits, Dirent is smaller. - ASSERT_EQ(sizeof(zim::writer::Dirent), 30); + ASSERT_EQ(sizeof(zim::writer::Dirent), 30U); #else - ASSERT_EQ(sizeof(zim::writer::Dirent), 38); + ASSERT_EQ(sizeof(zim::writer::Dirent), 38U); #endif #endif } diff --git a/test/dirent_lookup.cpp b/test/dirent_lookup.cpp index f3a11ea58..c0b461e72 100644 --- a/test/dirent_lookup.cpp +++ b/test/dirent_lookup.cpp @@ -74,24 +74,24 @@ class NamespaceBoundaryTest : public :: testing::Test TEST_F(NamespaceBoundaryTest, BeginOffset) { - ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'a').v, 10); - ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'b').v, 12); - ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'c').v, 13); - ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'A'-1).v, 0); - ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'A').v, 0); - ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'M').v, 9); - ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'U').v, 10); + ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'a').v, 10U); + ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'b').v, 12U); + ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'c').v, 13U); + ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'A'-1).v, 0U); + ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'A').v, 0U); + ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'M').v, 9U); + ASSERT_EQ(zim::getNamespaceBeginOffset(dirents, 'U').v, 10U); } TEST_F(NamespaceBoundaryTest, EndOffset) { - ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'a').v, 12); - ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'b').v, 13); - ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'c').v, 13); - ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'A'-1).v, 0); - ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'A').v, 9); - ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'M').v, 10); - ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'U').v, 10); + ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'a').v, 12U); + ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'b').v, 13U); + ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'c').v, 13U); + ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'A'-1).v, 0U); + ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'A').v, 9U); + ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'M').v, 10U); + ASSERT_EQ(zim::getNamespaceEndOffset(dirents, 'U').v, 10U); } TEST_F(NamespaceBoundaryTest, EndEqualsStartOfNext) @@ -151,10 +151,10 @@ TEST_F(DirentLookupTest, ExactMatch) CHECK_FIND_RESULT(expr, true, expected_value); \ CHECK_FIND_RESULT(fast_##expr, true, expected_value); - CHECK_EXACT_MATCH(direntLookup.find('A', "aa"), 0); - CHECK_EXACT_MATCH(direntLookup.find('a', "aa"), 10); - CHECK_EXACT_MATCH(direntLookup.find('A', "aabbbb"), 6); - CHECK_EXACT_MATCH(direntLookup.find('b', "aa"), 12); + CHECK_EXACT_MATCH(direntLookup.find('A', "aa"), 0U); + CHECK_EXACT_MATCH(direntLookup.find('a', "aa"), 10U); + CHECK_EXACT_MATCH(direntLookup.find('A', "aabbbb"), 6U); + CHECK_EXACT_MATCH(direntLookup.find('b', "aa"), 12U); #undef CHECK_EXACT_MATCH } @@ -169,17 +169,17 @@ TEST_F(DirentLookupTest, NoExactMatch) CHECK_FIND_RESULT(expr, false, expected_value); \ CHECK_FIND_RESULT(fast_##expr, false, expected_value); - CHECK_NOEXACT_MATCH(direntLookup.find('A', "ABC"), 0); - CHECK_NOEXACT_MATCH(direntLookup.find('U', "aa"), 10); // No U namespace => return 10 (the index of the first item from the next namespace) - CHECK_NOEXACT_MATCH(direntLookup.find('A', "aabb"), 5); // aabb is between aaaacc (4) and aabbaa (5) => 5 - CHECK_NOEXACT_MATCH(direntLookup.find('A', "aabbb"), 6); // aabbb is between aabbaa (5) and aabbbb (6) => 6 - CHECK_NOEXACT_MATCH(direntLookup.find('A', "aabbbc"), 7); // aabbbc is between aabbbb (6) and aabbcc (7) => 7 - CHECK_NOEXACT_MATCH(direntLookup.find('A', "bb"), 8); // bb is between aabbcc (7) and cccccc (8) => 8 - CHECK_NOEXACT_MATCH(direntLookup.find('A', "dd"), 9); // dd is after cccccc (8) => 9 - CHECK_NOEXACT_MATCH(direntLookup.find('M', "f"), 9); // f is before foo (9) => 9 - CHECK_NOEXACT_MATCH(direntLookup.find('M', "bar"), 9); // bar is before foo (9) => 9 - CHECK_NOEXACT_MATCH(direntLookup.find('M', "foo1"), 10); // foo1 is after foo (9) => 10 - CHECK_NOEXACT_MATCH(direntLookup.find('z', "zz"), 13); + CHECK_NOEXACT_MATCH(direntLookup.find('A', "ABC"), 0U); + CHECK_NOEXACT_MATCH(direntLookup.find('U', "aa"), 10U); // No U namespace => return 10 (the index of the first item from the next namespace) + CHECK_NOEXACT_MATCH(direntLookup.find('A', "aabb"), 5U); // aabb is between aaaacc (4) and aabbaa (5) => 5 + CHECK_NOEXACT_MATCH(direntLookup.find('A', "aabbb"), 6U); // aabbb is between aabbaa (5) and aabbbb (6) => 6 + CHECK_NOEXACT_MATCH(direntLookup.find('A', "aabbbc"), 7U); // aabbbc is between aabbbb (6) and aabbcc (7) => 7 + CHECK_NOEXACT_MATCH(direntLookup.find('A', "bb"), 8U); // bb is between aabbcc (7) and cccccc (8) => 8 + CHECK_NOEXACT_MATCH(direntLookup.find('A', "dd"), 9U); // dd is after cccccc (8) => 9 + CHECK_NOEXACT_MATCH(direntLookup.find('M', "f"), 9U); // f is before foo (9) => 9 + CHECK_NOEXACT_MATCH(direntLookup.find('M', "bar"), 9U); // bar is before foo (9) => 9 + CHECK_NOEXACT_MATCH(direntLookup.find('M', "foo1"), 10U); // foo1 is after foo (9) => 10 + CHECK_NOEXACT_MATCH(direntLookup.find('z', "zz"), 13U); #undef CHECK_NOEXACT_MATCH } diff --git a/test/find.cpp b/test/find.cpp index 68c6f3194..ea8fd99c8 100644 --- a/test/find.cpp +++ b/test/find.cpp @@ -80,7 +80,7 @@ TEST(FindTests, ByTitle) auto count = 0; for(auto& entry: range0) { count++; - ASSERT_EQ(entry.getTitle().find("Першая старонка"), 0); + ASSERT_EQ(entry.getTitle().find("Першая старонка"), 0U); } if (testfile.category == "withns") { // On the withns test file, there are two entry with this title: @@ -97,7 +97,7 @@ TEST(FindTests, ByTitle) count = 0; for(auto& entry: range1) { count++; - ASSERT_EQ(entry.getTitle().find("Украінская"), 0); + ASSERT_EQ(entry.getTitle().find("Украінская"), 0U); } ASSERT_EQ(count, 5); @@ -107,7 +107,7 @@ TEST(FindTests, ByTitle) count = 0; for(auto& entry: range2) { count++; - ASSERT_EQ(entry.getTitle().find("Украінская"), 0); + ASSERT_EQ(entry.getTitle().find("Украінская"), 0U); } ASSERT_EQ(count, 2); @@ -117,7 +117,7 @@ TEST(FindTests, ByTitle) count = 0; for(auto& entry: range3) { count++; - ASSERT_EQ(entry.getTitle().find("Украінская"), 0); + ASSERT_EQ(entry.getTitle().find("Украінская"), 0U); } ASSERT_EQ(count, 4); @@ -127,7 +127,7 @@ TEST(FindTests, ByTitle) count = 0; for(auto& entry: range4) { count++; - ASSERT_EQ(entry.getTitle().find("Украінская"), 0); + ASSERT_EQ(entry.getTitle().find("Украінская"), 0U); } ASSERT_EQ(count, 5); @@ -137,7 +137,7 @@ TEST(FindTests, ByTitle) count = 0; for(auto& entry: range5) { count++; - ASSERT_EQ(entry.getTitle().find("Украінская"), 0); + ASSERT_EQ(entry.getTitle().find("Украінская"), 0U); } ASSERT_EQ(count, 0); } @@ -149,7 +149,7 @@ TEST(FindTests, ByTitle) auto range = archive.findByTitle(prefix); \ for(auto& entry: range) { \ count++; \ - ASSERT_EQ(entry.getTitle().find(prefix), 0); \ + ASSERT_EQ(entry.getTitle().find(prefix), 0U); \ } \ ASSERT_EQ(count, expected_count); \ } @@ -193,62 +193,62 @@ TEST(FindTests, ByPath) auto range5 = archive.findByPath(""); auto range6 = archive.findByPath("/"); - ASSERT_EQ(range0.begin()->getIndex(), 5); - auto count = 0; + ASSERT_EQ(range0.begin()->getIndex(), 5U); + unsigned count = 0; for(auto& entry: range0) { count++; - ASSERT_EQ(entry.getPath().find("A/Main_Page.html"), 0); + ASSERT_EQ(entry.getPath().find("A/Main_Page.html"), 0U); } - ASSERT_EQ(count, 1); + ASSERT_EQ(count, 1U); - ASSERT_EQ(range1.begin()->getIndex(), 78); + ASSERT_EQ(range1.begin()->getIndex(), 78U); count = 0; for(auto& entry: range1) { count++; std::cout << entry.getPath() << std::endl; - ASSERT_EQ(entry.getPath().find("I/s/"), 0); + ASSERT_EQ(entry.getPath().find("I/s/"), 0U); } - ASSERT_EQ(count, 31); + ASSERT_EQ(count, 31U); - ASSERT_EQ(range2.begin()->getIndex(), 2); + ASSERT_EQ(range2.begin()->getIndex(), 2U); count = 0; for(auto& entry: range2) { count++; - ASSERT_EQ(entry.getPath().find("-/j/head.js"), 0); + ASSERT_EQ(entry.getPath().find("-/j/head.js"), 0U); } - ASSERT_EQ(count, 1); + ASSERT_EQ(count, 1U); - ASSERT_EQ(range3.begin()->getIndex(), 75); + ASSERT_EQ(range3.begin()->getIndex(), 75U); count = 0; for(auto& entry: range3) { count++; std::cout << entry.getPath() << std::endl; - ASSERT_EQ(entry.getPath().find("I"), 0); + ASSERT_EQ(entry.getPath().find("I"), 0U); } - ASSERT_EQ(count, 34); + ASSERT_EQ(count, 34U); - ASSERT_EQ(range4.begin()->getIndex(), 75); + ASSERT_EQ(range4.begin()->getIndex(), 75U); count = 0; for(auto& entry: range4) { count++; std::cout << entry.getPath() << std::endl; - ASSERT_EQ(entry.getPath().find("I/"), 0); + ASSERT_EQ(entry.getPath().find("I/"), 0U); } - ASSERT_EQ(count, 34); + ASSERT_EQ(count, 34U); count = 0; for(auto& entry: range5) { ASSERT_EQ(count, entry.getIndex()); count++; } - ASSERT_EQ(count, 118); + ASSERT_EQ(count, 118U); count = 0; for(auto& entry: range6) { ASSERT_EQ(count, entry.getIndex()); count++; } - ASSERT_EQ(count, 118); + ASSERT_EQ(count, 118U); } } @@ -263,34 +263,34 @@ TEST(FindTests, ByPathNons) auto range2 = archive.findByPath(""); auto range3 = archive.findByPath("/"); - auto count = 0; + unsigned count = 0; for(auto& entry: range0) { count++; - ASSERT_EQ(entry.getPath().find("Першая_старонка.html"), 0); + ASSERT_EQ(entry.getPath().find("Першая_старонка.html"), 0U); } - ASSERT_EQ(count, 1); + ASSERT_EQ(count, 1U); count = 0; for(auto& entry: range1) { count++; std::cout << entry.getPath() << std::endl; - ASSERT_EQ(entry.getPath().find("П"), 0); + ASSERT_EQ(entry.getPath().find("П"), 0U); } - ASSERT_EQ(count, 2); + ASSERT_EQ(count, 2U); count = 0; for(auto& entry: range2) { ASSERT_EQ(count, entry.getIndex()); count++; } - ASSERT_EQ(count, 109); + ASSERT_EQ(count, 109U); count = 0; for(auto& entry: range3) { ASSERT_EQ(count, entry.getIndex()); count++; } - ASSERT_EQ(count, 109); + ASSERT_EQ(count, 109U); } } #endif diff --git a/test/lrucache.cpp b/test/lrucache.cpp index 47daa516c..fff97717e 100644 --- a/test/lrucache.cpp +++ b/test/lrucache.cpp @@ -34,14 +34,14 @@ #include "gtest/gtest.h" const int NUM_OF_TEST2_RECORDS = 100; -const int TEST2_CACHE_CAPACITY = 50; +const unsigned int TEST2_CACHE_CAPACITY = 50u; TEST(CacheTest, SimplePut) { zim::lru_cache cache_lru(1); cache_lru.put(7, 777); EXPECT_TRUE(cache_lru.exists(7)); EXPECT_EQ(777, cache_lru.get(7)); - EXPECT_EQ(1, cache_lru.size()); + EXPECT_EQ(1u, cache_lru.size()); } TEST(CacheTest, OverwritingPut) { @@ -50,7 +50,7 @@ TEST(CacheTest, OverwritingPut) { cache_lru.put(7, 222); EXPECT_TRUE(cache_lru.exists(7)); EXPECT_EQ(222, cache_lru.get(7)); - EXPECT_EQ(1, cache_lru.size()); + EXPECT_EQ(1u, cache_lru.size()); } TEST(CacheTest, MissingValue) { @@ -65,13 +65,13 @@ TEST(CacheTest, DropValue) { cache_lru.put(7, 777); cache_lru.put(8, 888); cache_lru.put(9, 999); - EXPECT_EQ(3, cache_lru.size()); + EXPECT_EQ(3u, cache_lru.size()); EXPECT_TRUE(cache_lru.exists(7)); EXPECT_EQ(777, cache_lru.get(7)); EXPECT_TRUE(cache_lru.drop(7)); - EXPECT_EQ(2, cache_lru.size()); + EXPECT_EQ(2u, cache_lru.size()); EXPECT_FALSE(cache_lru.exists(7)); EXPECT_THROW(cache_lru.get(7).value(), std::range_error); @@ -85,7 +85,7 @@ TEST(CacheTest1, KeepsAllValuesWithinCapacity) { cache_lru.put(i, i); } - for (int i = 0; i < NUM_OF_TEST2_RECORDS - TEST2_CACHE_CAPACITY; ++i) { + for (unsigned i = 0; i < NUM_OF_TEST2_RECORDS - TEST2_CACHE_CAPACITY; ++i) { EXPECT_FALSE(cache_lru.exists(i)); } diff --git a/test/random.cpp b/test/random.cpp index cedb24882..d18a567c2 100644 --- a/test/random.cpp +++ b/test/random.cpp @@ -31,13 +31,13 @@ namespace TEST(Random, smallMax) { for(auto i=0; i<1000; i++) { - ASSERT_EQ(randomNumber(0), 0); + ASSERT_EQ(randomNumber(0), 0U); } for(auto i=0; i<1000; i++) { auto r = randomNumber(1); - ASSERT_TRUE(r>=0 && r<=1) << r; + ASSERT_TRUE(r>=0U && r<=1U) << r; } } diff --git a/test/rawstreamreader.cpp b/test/rawstreamreader.cpp index ec1bdb41b..226d6e57e 100644 --- a/test/rawstreamreader.cpp +++ b/test/rawstreamreader.cpp @@ -43,12 +43,12 @@ TEST(ReaderDataStreamWrapper, shouldJustWork) RawStreamReader rdr(reader); - ASSERT_EQ(1234, rdr.read()); + ASSERT_EQ(1234U, rdr.read()); auto subbuffer = rdr.sub_reader(zsize_t(4))->get_buffer(offset_t(0), zsize_t(4)); ASSERT_EQ("efgh", toString(subbuffer)); subbuffer = rdr.sub_reader(zsize_t(10))->get_buffer(offset_t(0), zsize_t(10)); ASSERT_EQ("ijklmnopqr", toString(subbuffer)); - ASSERT_EQ(-987654321, rdr.read()); + ASSERT_EQ(-987654321, rdr.read()); } } // unnamed namespace diff --git a/test/search.cpp b/test/search.cpp index d98e47ee2..5e6093ec3 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -133,7 +133,7 @@ TEST(Search, multiSearch) zim::Query query("test article"); auto search0 = searcher.search(query); - ASSERT_EQ(archive.getEntryCount(), search0.getEstimatedMatches()); + ASSERT_EQ(archive.getEntryCount(), (unsigned int)search0.getEstimatedMatches()); auto result0 = search0.getResults(0, 2); ASSERT_EQ(result0.size(), 2); auto it0 = result0.begin(); @@ -214,7 +214,7 @@ TEST(Search, noStemming) zim::Query query("test article"); auto search = searcher.search(query); - ASSERT_EQ(archive.getEntryCount(), search.getEstimatedMatches()); + ASSERT_EQ(archive.getEntryCount(), (unsigned int)search.getEstimatedMatches()); auto result = search.getResults(0, 1); ASSERT_EQ(result.begin().getTitle(), "Test Article0"); } @@ -241,7 +241,7 @@ TEST(Search, geoQuery) query.setGeorange(45.000, 10.000, 100); auto search = searcher.search(query); - ASSERT_EQ(archive.getEntryCount(), search.getEstimatedMatches()); + ASSERT_EQ(archive.getEntryCount(), (unsigned int)search.getEstimatedMatches()); auto result = search.getResults(0, 1); ASSERT_EQ(result.begin().getTitle(), "Test Article"); } @@ -310,7 +310,7 @@ TEST(Search, accents) zim::Query query("test article"); auto search = searcher.search(query); - ASSERT_EQ(archive.getEntryCount(), search.getEstimatedMatches()); + ASSERT_EQ(archive.getEntryCount(), (unsigned int)search.getEstimatedMatches()); auto result = search.getResults(0, 1); ASSERT_EQ(result.begin().getTitle(), "Test Article0"); } @@ -319,7 +319,7 @@ TEST(Search, accents) zim::Query query("test àrticlé"); auto search = searcher.search(query); - ASSERT_EQ(archive.getEntryCount(), search.getEstimatedMatches()); + ASSERT_EQ(archive.getEntryCount(), (unsigned int)search.getEstimatedMatches()); auto result = search.getResults(0, 1); ASSERT_EQ(result.begin().getTitle(), "Test Article0"); } diff --git a/test/tinyString.cpp b/test/tinyString.cpp index 5ef6ce62c..6d0641331 100644 --- a/test/tinyString.cpp +++ b/test/tinyString.cpp @@ -30,7 +30,7 @@ TEST(TinyStringTest, empty) { TinyString s; ASSERT_TRUE(s.empty()); - ASSERT_EQ(s.size(), 0); + ASSERT_EQ(s.size(), 0U); ASSERT_EQ((std::string)s, ""); ASSERT_EQ(s, TinyString()); } @@ -39,7 +39,7 @@ TEST(TinyStringTest, noChar) { TinyString s(""); ASSERT_TRUE(s.empty()); - ASSERT_EQ(s.size(), 0); + ASSERT_EQ(s.size(), 0U); ASSERT_EQ((std::string)s, ""); ASSERT_EQ(s, TinyString()); } @@ -48,7 +48,7 @@ TEST(TinyStringTest, oneChar) { TinyString s("A"); ASSERT_FALSE(s.empty()); - ASSERT_EQ(s.size(), 1); + ASSERT_EQ(s.size(), 1U); ASSERT_EQ((std::string)s, "A"); ASSERT_TRUE(s < TinyString("B")); ASSERT_EQ(s, TinyString("A")); @@ -59,7 +59,7 @@ TEST(TinyStringTest, chars) { TinyString s("ABCDE"); ASSERT_FALSE(s.empty()); - ASSERT_EQ(s.size(), 5); + ASSERT_EQ(s.size(), 5U); ASSERT_EQ((std::string)s, "ABCDE"); ASSERT_FALSE(s < TinyString()); ASSERT_FALSE(s < TinyString("")); @@ -79,7 +79,7 @@ TEST(PathTitleTinyString, none) { PathTitleTinyString s; ASSERT_TRUE(s.empty()); - ASSERT_EQ(s.size(), 0); + ASSERT_EQ(s.size(), 0U); ASSERT_EQ((std::string)s, ""); ASSERT_EQ(s, TinyString()); ASSERT_EQ(s.getPath(), ""); @@ -92,7 +92,7 @@ TEST(PathTitleTinyString, empty) //We have the separator between path and title PathTitleTinyString s("", ""); ASSERT_FALSE(s.empty()); - ASSERT_EQ(s.size(), 1); + ASSERT_EQ(s.size(), 1U); ASSERT_EQ((std::string)s, std::string("", 1)); ASSERT_EQ(s.getPath(), ""); ASSERT_EQ(s.getTitle(false), ""); @@ -104,7 +104,7 @@ TEST(PathTitleTinyString, no_title) //We have the separator between path and title PathTitleTinyString s("FOO", ""); ASSERT_FALSE(s.empty()); - ASSERT_EQ(s.size(), 4); + ASSERT_EQ(s.size(), 4U); ASSERT_EQ((std::string)s, std::string("FOO\0", 4)); ASSERT_EQ(s.getPath(), "FOO"); ASSERT_EQ(s.getTitle(false), "FOO"); @@ -116,7 +116,7 @@ TEST(PathTitleTinyString, no_path) //We have the separator between path and title PathTitleTinyString s("", "BAR"); ASSERT_FALSE(s.empty()); - ASSERT_EQ(s.size(), 4); + ASSERT_EQ(s.size(), 4U); ASSERT_EQ((std::string)s, std::string("\0BAR", 4)); ASSERT_EQ(s.getPath(), ""); ASSERT_EQ(s.getTitle(false), "BAR"); @@ -128,7 +128,7 @@ TEST(PathTitleTinyString, path_title) //We have the separator between path and title PathTitleTinyString s("FOO", "BAR"); ASSERT_FALSE(s.empty()); - ASSERT_EQ(s.size(), 7); + ASSERT_EQ(s.size(), 7U); ASSERT_EQ((std::string)s, std::string("FOO\0BAR", 7)); ASSERT_EQ(s.getPath(), "FOO"); ASSERT_EQ(s.getTitle(false), "BAR"); @@ -140,7 +140,7 @@ TEST(PathTitleTinyString, equal_path_title) //We have the separator between path and title PathTitleTinyString s("FOO", "FOO"); ASSERT_FALSE(s.empty()); - ASSERT_EQ(s.size(), 4); + ASSERT_EQ(s.size(), 4U); ASSERT_EQ((std::string)s, std::string("FOO\0", 4)); ASSERT_EQ(s.getPath(), "FOO"); ASSERT_EQ(s.getTitle(false), "FOO"); diff --git a/test/tooltesting.cpp b/test/tooltesting.cpp index b6329c4cb..aa46463b2 100644 --- a/test/tooltesting.cpp +++ b/test/tooltesting.cpp @@ -29,23 +29,23 @@ namespace { TEST(Tools, wordCount) { - ASSERT_EQ(zim::countWords(""), 0); - ASSERT_EQ(zim::countWords(" "), 0); - ASSERT_EQ(zim::countWords("One"), 1); - ASSERT_EQ(zim::countWords("One Two Three"), 3); - ASSERT_EQ(zim::countWords(" One "), 1); - ASSERT_EQ(zim::countWords("One Two Three "), 3); - ASSERT_EQ(zim::countWords("One.Two\tThree"), 2); + ASSERT_EQ(zim::countWords(""), 0U); + ASSERT_EQ(zim::countWords(" "), 0U); + ASSERT_EQ(zim::countWords("One"), 1U); + ASSERT_EQ(zim::countWords("One Two Three"), 3U); + ASSERT_EQ(zim::countWords(" One "), 1U); + ASSERT_EQ(zim::countWords("One Two Three "), 3U); + ASSERT_EQ(zim::countWords("One.Two\tThree"), 2U); } TEST(Tools, parseIllustrationPathToSize) { - ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_0x0@1"), 0); - ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_1x1@1"), 1); - ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_01x01@1"), 1); - ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_64x64@1"), 64); - ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_128x128@1"), 128); - ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_1024x1024@1"), 1024); + ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_0x0@1"), 0U); + ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_1x1@1"), 1U); + ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_01x01@1"), 1U); + ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_64x64@1"), 64U); + ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_128x128@1"), 128U); + ASSERT_EQ(zim::parseIllustrationPathToSize("Illustration_1024x1024@1"), 1024U); ASSERT_THROW(zim::parseIllustrationPathToSize("Illsration_64x64@1"), std::runtime_error); ASSERT_THROW(zim::parseIllustrationPathToSize("Illstration_"), std::runtime_error); ASSERT_THROW(zim::parseIllustrationPathToSize("Illustration_64x@1"), std::runtime_error); From 9c888428744d9fa6e8f813fd0ec14b1278d41859 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 30 Aug 2023 19:40:50 +0200 Subject: [PATCH 06/12] Remove unused variable. --- src/cluster.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cluster.cpp b/src/cluster.cpp index 32afd8d41..7ac93e359 100644 --- a/src/cluster.cpp +++ b/src/cluster.cpp @@ -106,7 +106,6 @@ getClusterReader(const Reader& zimReader, offset_t offset, Cluster::Compression* OFFSET_TYPE offset = m_reader->read(); size_t n_offset = offset / sizeof(OFFSET_TYPE); - const offset_t data_address(offset); // read offsets m_blobOffsets.clear(); From 163b02bac82c38c3e8aac38b2188cead7386f31b Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Thu, 31 Aug 2023 09:26:56 +0200 Subject: [PATCH 07/12] No implicit conversion `double` to `int`. --- src/tools.cpp | 2 +- test/error_in_creator.cpp | 4 ++-- test/random.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools.cpp b/src/tools.cpp index 0e001d92a..58894f384 100644 --- a/src/tools.cpp +++ b/src/tools.cpp @@ -125,7 +125,7 @@ uint32_t zim::randomNumber(uint32_t max) static std::mutex mutex; std::lock_guard l(mutex); - return ((double)random() / random.max()) * max; + return (uint32_t)(((double)random() / random.max()) * max); } /* Split string in a token array */ diff --git a/test/error_in_creator.cpp b/test/error_in_creator.cpp index 163024a25..e980a39ce 100644 --- a/test/error_in_creator.cpp +++ b/test/error_in_creator.cpp @@ -288,7 +288,7 @@ TEST_P(FaultyDelayedItemErrorTest, faultyCompressedItem) // The exact value is specific to each computer, so we need to make this configurable. // We use a base and we multiply it by a factor taken from env variable. const long sleep_time = 1000000; // Default value is set to a factor 10 above what is needed to work on my (fast) computer - zim::microsleep(sleep_time * getWaitTimeFactor()); + zim::microsleep((int)(sleep_time * getWaitTimeFactor())); // We detect it for any call after CHECK_ASYNC_EXCEPT(creator.addMetadata("Title", "This is a title")); CHECK_ASYNC_EXCEPT(creator.addIllustration(48, "PNGBinaryContent48")); @@ -320,7 +320,7 @@ TEST_P(FaultyDelayedItemErrorTest, faultyUnCompressedItem) // Note here, that we have a base smaller than for compressed test as we don't compress the content // and the writer thread (the one using the contentProvider) detect the error sooner const long sleep_time = 10000; // Default value is set to a factor 10 above what is needed to work on my (fast) computer - zim::microsleep(sleep_time * getWaitTimeFactor()); + zim::microsleep((int)(sleep_time * getWaitTimeFactor())); // But we detect it for any call after CHECK_ASYNC_EXCEPT(creator.addMetadata("Title", "This is a title")); CHECK_ASYNC_EXCEPT(creator.addIllustration(48, "PNGBinaryContent48")); diff --git a/test/random.cpp b/test/random.cpp index d18a567c2..8a45303af 100644 --- a/test/random.cpp +++ b/test/random.cpp @@ -51,7 +51,7 @@ TEST(Random, distribution) for (auto i=0U; i Date: Thu, 31 Aug 2023 09:41:41 +0200 Subject: [PATCH 08/12] Explicit cast of `-1` to `zsize_t` --- src/writer/contentProvider.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/writer/contentProvider.cpp b/src/writer/contentProvider.cpp index 0896d90c9..48f0c2d9a 100644 --- a/src/writer/contentProvider.cpp +++ b/src/writer/contentProvider.cpp @@ -63,7 +63,7 @@ namespace zim return Blob(nullptr, 0); } - if(fd->readAt(buffer.get(), zim::zsize_t(sizeToRead), zim::offset_t(offset)).v == -1UL) { + if(fd->readAt(buffer.get(), zim::zsize_t(sizeToRead), zim::offset_t(offset)) == zim::zsize_t(-1)) { throw std::runtime_error("Error reading file " + filepath); } offset += sizeToRead; From 94bd1db40509b1a023022e9cada519f8ec5d7105 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Thu, 31 Aug 2023 10:48:24 +0200 Subject: [PATCH 09/12] No ignoring return value --- test/tools.cpp | 4 +++- test/tools.h | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/test/tools.cpp b/test/tools.cpp index 2013ee222..7d9e18c3e 100644 --- a/test/tools.cpp +++ b/test/tools.cpp @@ -95,7 +95,9 @@ std::unique_ptr makeTempFile(const char* name, const std::string& content) { std::unique_ptr p(new TempFile(name)); - write(p->fd(), &content[0], content.size()); + if (write(p->fd(), &content[0], content.size()) != (ssize_t)content.size()) { + throw std::runtime_error("Error writing temp file " + p->path()); + } p->close(); return p; } diff --git a/test/tools.h b/test/tools.h index f5aa2eb59..5ca1e2398 100644 --- a/test/tools.h +++ b/test/tools.h @@ -27,7 +27,9 @@ #ifdef _WIN32 #include #include +#include #define LSEEK _lseeki64 +typedef SSIZE_T ssize_t; #else #include #define LSEEK lseek @@ -113,7 +115,9 @@ zim::Buffer write_to_buffer(const T& object, const std::string& tail="") TempFile tmpFile("test_temp_file"); const auto tmp_fd = tmpFile.fd(); object.write(tmp_fd); - write(tmp_fd, tail.data(), tail.size()); + if (write(tmp_fd, tail.data(), tail.size()) != (ssize_t)tail.size()) { + throw std::runtime_error("Cannot write to " + tmpFile.path()); + } size_type size = LSEEK(tmp_fd, 0, SEEK_END); auto buf = zim::Buffer::makeBuffer(zim::zsize_t(size)); From 681fde1fdcf885a8228bc489348999ff9fe3cca1 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Thu, 31 Aug 2023 11:02:27 +0200 Subject: [PATCH 10/12] Do not try to build subproject `zstd` with `werror`. --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index ca0fe5b8f..f28595599 100644 --- a/meson.build +++ b/meson.build @@ -41,7 +41,7 @@ else public_conf.set('LIBZIM_EXPORT_DLL', true) endif -zstd_dep = dependency('libzstd', static:static_linkage) +zstd_dep = dependency('libzstd', static:static_linkage, default_options:['werror=false']) if host_machine.system() == 'freebsd' execinfo_dep = cpp.find_library('execinfo') From 9b219191d3374f9b54cec225c81428e5f688081f Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Thu, 31 Aug 2023 11:08:36 +0200 Subject: [PATCH 11/12] Fix the lib_postfix for aarch64 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 753b15d21..451030fbb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -128,7 +128,7 @@ jobs: lib_postfix: '/x86_64-linux-musl' - target: aarch64_dyn image_variant: focal - lib_postfix: '/aarch64-rpi3-linux-gnu' + lib_postfix: '/aarch64-linux-gnu' - target: android_arm image_variant: focal lib_postfix: '/arm-linux-androideabi' From 51947a86d5f10913b1bc16a2363b139053083cda Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Thu, 31 Aug 2023 11:23:00 +0200 Subject: [PATCH 12/12] Define function use with `WITH_TEST_DATA` only if `WITH_TEST_DATA`. Or else, they will be detected as defined but not used. --- test/archive.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/archive.cpp b/test/archive.cpp index fb172834c..ee6eee983 100644 --- a/test/archive.cpp +++ b/test/archive.cpp @@ -481,7 +481,7 @@ TEST(ZimArchive, validate) EXPECT_BROKEN_ZIMFILE(testfile.path, expected) } } -#endif + void checkEquivalence(const zim::Archive& archive1, const zim::Archive& archive2) { @@ -559,7 +559,6 @@ void checkEquivalence(const zim::Archive& archive1, const zim::Archive& archive2 #endif } -#if WITH_TEST_DATA TEST(ZimArchive, multipart) { auto nonSplittedZims = getDataFilePath("wikibooks_be_all_nopic_2017-02.zim"); @@ -617,6 +616,8 @@ TEST(ZimArchive, openZIMFileEmbeddedInAnotherFile) #endif // not _WIN32 #endif // WITH_TEST_DATA + +#if WITH_TEST_DATA zim::Blob readItemData(const zim::Item::DirectAccessInfo& dai, zim::size_type size) { zim::DEFAULTFS::FD fd(zim::DEFAULTFS::openFile(dai.first)); @@ -625,7 +626,6 @@ zim::Blob readItemData(const zim::Item::DirectAccessInfo& dai, zim::size_type si return zim::Blob(data, size); } -#if WITH_TEST_DATA TEST(ZimArchive, getDirectAccessInformation) { for(auto& testfile:getDataFilePath("small.zim")) {