Merge pull request #819 from openzim/cpp17

.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'

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<EntryOrder order>
-  class LIBZIM_API Archive::iterator : public std::iterator<std::bidirectional_iterator_tag, Entry>
+  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<FileImpl> file, entry_index_type idx)
         : m_file(file),
           m_idx(idx),

include/zim/search_iterator.h

@@ -32,10 +32,34 @@ namespace zim
 {
 class SearchResultSet;
 
-class LIBZIM_API SearchIterator : public std::iterator<std::bidirectional_iterator_tag, Entry>
+/**
+ * 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);

include/zim/suggestion_iterator.h

@@ -31,11 +31,36 @@ class SuggestionResultSet;
 class SuggestionItem;
 class SearchIterator;
 
-class LIBZIM_API SuggestionIterator : public std::iterator<std::bidirectional_iterator_tag, SuggestionItem>
+/**
+ * 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<EntryOrder::titleOrder> 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);

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', 'werror=true'])
 
 if build_machine.system() != 'windows'
   add_project_arguments('-D_LARGEFILE64_SOURCE=1', '-D_FILE_OFFSET_BITS=64', language: 'cpp')
@@ -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')

src/cluster.cpp

@@ -106,7 +106,6 @@ getClusterReader(const Reader& zimReader, offset_t offset, Cluster::Compression*
     OFFSET_TYPE offset = m_reader->read<OFFSET_TYPE>();
 
     size_t n_offset = offset / sizeof(OFFSET_TYPE);
-    const offset_t data_address(offset);
 
     // read offsets
     m_blobOffsets.clear();

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);

src/tools.cpp

@@ -125,7 +125,7 @@ uint32_t zim::randomNumber(uint32_t max)
   static std::mutex mutex;
 
   std::lock_guard<std::mutex> l(mutex);
-  return ((double)random() / random.max()) * max;
+  return (uint32_t)(((double)random() / random.max()) * max);
 }
 
 /* Split string in a token array */

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;

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<uint8_t>(ns)),
     removed(false),

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<std::string>({"Counter", "Illustration_48x48@1", "Illustration_96x96@1", "Title"}));
   ASSERT_EQ(archive.getIllustrationSizes(), std::set<unsigned int>({48, 96}));
@@ -481,7 +481,7 @@ TEST(ZimArchive, validate)
     EXPECT_BROKEN_ZIMFILE(testfile.path, expected)
   }
 }
-#endif
+
 
 void checkEquivalence(const zim::Archive& archive1, const zim::Archive& archive2)
 {
@@ -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();
@@ -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")) {
@@ -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

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<uint32_t>());
+  ASSERT_EQ(1234U, bds.read<uint32_t>());
 
   ASSERT_EQ(data + 4, bds.current());
   const auto blob1 = std::string(bds.current(), 4);

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) {
   {

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()));

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);

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
 }