archive_sys refactoring: ArchiveEntryView

Entries in libarchive archives are forward iterate only. Entries are owned by
the current archive. Reading the next header can hence invalidate the current
entry.

ArchiveEntrySys was replaced by ArchiveEntryView. ArchiveEntryView now only
guarantees a temporary view of the currently active archive entry.

src/archive.cpp

@@ -18,7 +18,7 @@ namespace logger = spdlog;
 namespace xwim {
 
 static void _spec_is_root_filename(ArchiveSpec* spec,
-                                   ArchiveEntrySys& entry,
+                                   ArchiveEntryView entry,
                                    std::filesystem::path* filepath) {
   auto entry_path = entry.path();
   auto norm_stem = filepath->filename();
@@ -37,7 +37,7 @@ static void _spec_is_root_filename(ArchiveSpec* spec,
   logger::debug("\t-> Archive stem: {}", norm_stem.string());
 }
 
-static void _spec_is_root_dir(ArchiveSpec* spec, ArchiveEntrySys& entry) {
+static void _spec_is_root_dir(ArchiveSpec* spec, ArchiveEntryView entry) {
   if (entry.is_directory()) {
     logger::debug("Archive root is directory");
     spec->is_root_dir = true;
@@ -49,16 +49,15 @@ static void _spec_is_root_dir(ArchiveSpec* spec, ArchiveEntrySys& entry) {
 }
 
 static void _spec_has_single_root(ArchiveSpec* spec,
-                                  ArchiveEntrySys& first_entry,
+                                  ArchiveEntryView first_entry,
                                   ArchiveReaderSys& archive_reader) {
   std::filesystem::path first_entry_root = *(first_entry.path().begin());
   logger::trace("Testing roots");
 
   spec->has_single_root = true;
 
-  while (true) {
+  while (archive_reader.advance()) {
-    ArchiveEntrySys& entry = archive_reader.next();
+    ArchiveEntryView entry = archive_reader.cur();
-    if(entry.is_empty()) break;
 
     auto next_entry = entry.path();
     logger::trace("Path: {}, Root: {}", next_entry.string(),
@@ -92,13 +91,13 @@ ArchiveSpec Archive::check() {
 
   ArchiveSpec archive_spec;
 
-  ArchiveEntrySys& first_entry = archive_reader.next();
+  if (!archive_reader.advance()) { // can't advance even once, archive is empty
-
-  if (first_entry.is_empty()) {  // archive is empty
     logger::debug("Archive is empty");
     return {false, false, false};
   }
 
+  ArchiveEntryView first_entry = archive_reader.cur();
+
   logger::trace("Found archive entry {}", first_entry.path_name());
 
   _spec_is_root_filename(&archive_spec, first_entry, &this->path);
@@ -109,6 +108,9 @@ ArchiveSpec Archive::check() {
 }
 
 void Archive::extract(ExtractSpec extract_spec) {
+  std::filesystem::path abs_path = std::filesystem::absolute(this->path);
+  ArchiveReaderSys reader{abs_path};
+
   ArchiveExtractorSys extractor;
 
   if(extract_spec.make_dir) {
@@ -118,7 +120,6 @@ void Archive::extract(ExtractSpec extract_spec) {
     extractor = ArchiveExtractorSys{};
   }
 
-  ArchiveReaderSys reader{this->path};
   extractor.extract_all(reader);
 }
 

src/archive_sys.cpp

@@ -8,23 +8,27 @@ namespace logger = spdlog;
 #include <filesystem>
 #include <memory>
 
-bool xwim::ArchiveEntrySys::is_empty() {
+bool xwim::ArchiveEntryView::is_empty() {
-  return (this->ae.get() == nullptr);
+  return (this->ae == nullptr);
 }
 
-std::string xwim::ArchiveEntrySys::path_name() {
+std::string xwim::ArchiveEntryView::path_name() {
-  return archive_entry_pathname(this->ae.get());
+  if (!this->ae) throw ArchiveSysException{"Access to invalid archive entry"};
+
+  return archive_entry_pathname(this->ae);
 }
 
-std::filesystem::path xwim::ArchiveEntrySys::path() {
+std::filesystem::path xwim::ArchiveEntryView::path() {
+  if (!this->ae) throw ArchiveSysException{"Access to invalid archive entry"};
   return std::filesystem::path{this->path_name()};
 }
 
-mode_t xwim::ArchiveEntrySys::file_type() {
+mode_t xwim::ArchiveEntryView::file_type() {
-  return archive_entry_filetype(this->ae.get());
+  if (!this->ae) throw ArchiveSysException{"Access to invalid archive entry"};
+  return archive_entry_filetype(this->ae);
 }
 
-bool xwim::ArchiveEntrySys::is_directory() {
+bool xwim::ArchiveEntryView::is_directory() {
   return S_ISDIR(this->file_type());
 }
 
@@ -48,24 +52,20 @@ xwim::ArchiveReaderSys::~ArchiveReaderSys() {
   archive_free(this->ar);
 }
 
-xwim::ArchiveEntrySys& xwim::ArchiveReaderSys::next() {
+bool xwim::ArchiveReaderSys::advance() {
   int r;  // libarchive error handling
-  logger::trace("Listing next archive entry");
+  logger::trace("Advancing reader to next archive entry");
-  archive_entry* ae;
-  r = archive_read_next_header(this->ar, &ae);
-  if (r != ARCHIVE_OK)
-    throw(ArchiveSysException{"Could not list archive", this->ar});
 
-  this->cur_entry = xwim::ArchiveEntrySys { ae };
+  r = archive_read_next_header(this->ar, &this->ae);
+  if (r == ARCHIVE_EOF) { this->ae = nullptr; return false; }
+  if (r != ARCHIVE_OK) throw(ArchiveSysException{"Could not list archive", this->ar});
 
-
+  logger::trace("Got entry {}", archive_entry_pathname(ae));
-  logger::trace("Got archive header");
+  return true;
-
-  return this->cur_entry;
 }
 
-xwim::ArchiveEntrySys& xwim::ArchiveReaderSys::cur() {
+const xwim::ArchiveEntryView xwim::ArchiveReaderSys::cur() {
-  return this->cur_entry;
+  return ArchiveEntryView{this->ae};
 }
 
 xwim::ArchiveExtractorSys::ArchiveExtractorSys(std::filesystem::path& root) {
@@ -82,12 +82,24 @@ xwim::ArchiveExtractorSys::ArchiveExtractorSys() {
 }
 
 void xwim::ArchiveExtractorSys::extract_all(xwim::ArchiveReaderSys& reader) {
-  for(;;) {
+  while(reader.advance()) {
-    ArchiveEntrySys& entry = reader.next();
+    this->extract_entry(reader);
+  }
+}
 
-    if(entry.is_empty()) break;
+// forward declared
+static int copy_data(struct archive* ar, struct archive* aw);
 
-    this->extract(reader, entry);
+void xwim:: ArchiveExtractorSys::extract_entry(xwim::ArchiveReaderSys& reader) {
+  int r;
+  r = archive_write_header(this->writer, reader.ae);
+  if (r != ARCHIVE_OK) {
+    throw(ArchiveSysException("Could not extract entry", reader.ar));
+  }
+
+  r = copy_data(reader.ar, this->writer);
+  if (r != ARCHIVE_OK) {
+    throw(ArchiveSysException("Could not extract entry", reader.ar));
   }
 }
 
@@ -110,16 +122,3 @@ static int copy_data(struct archive* ar, struct archive* aw) {
     }
   }
 }
-
-void xwim::ArchiveExtractorSys::extract(xwim::ArchiveReaderSys& reader, xwim::ArchiveEntrySys& entry) {
-  int r;
-  r = archive_write_header(this->writer, entry.ae.get());
-  if (r != ARCHIVE_OK) {
-    throw(ArchiveSysException("Could not extract entry", reader.ar));
-  }
-
-  r = copy_data(reader.ar, this->writer);
-  if (r != ARCHIVE_OK) {
-    throw(ArchiveSysException("Could not extract entry", reader.ar));
-  }
-}

src/archive_sys.hpp

@@ -7,22 +7,13 @@
 
 namespace xwim {
 
-class ArchiveEntrySys {
+class ArchiveEntryView {
  private:
-  std::function<void (archive_entry*)> ae_deleter = [](archive_entry* ae) { archive_entry_free(ae); };
+  archive_entry* ae;
-  std::unique_ptr<archive_entry, decltype(ae_deleter)> ae;
-  friend class ArchiveExtractorSys;
 
  public:
-  ArchiveEntrySys(std::unique_ptr<archive_entry> entry)
+  ArchiveEntryView() = default;
-    : ae{ std::unique_ptr<archive_entry, decltype(ae_deleter)>{entry.release(), ae_deleter} }
+  ArchiveEntryView(archive_entry* entry) : ae{entry} {}
-  {}
-  ArchiveEntrySys()
-    : ae{ std::unique_ptr<archive_entry, decltype(ae_deleter)>{nullptr, ae_deleter} }
-  {}
-  ArchiveEntrySys(archive_entry* entry)
-    : ae{ std::unique_ptr<archive_entry, decltype(ae_deleter)>{entry, ae_deleter} }
-  {}
 
   bool is_empty();
   std::string path_name();
@@ -39,24 +30,31 @@ class ArchiveEntrySys {
 class ArchiveReaderSys {
  private:
   archive* ar;
-  ArchiveEntrySys cur_entry;
+  archive_entry* ae;
   friend class ArchiveExtractorSys;
 
  public:
   ArchiveReaderSys(std::filesystem::path& path);
   ~ArchiveReaderSys();
 
-  /** Returns the next archive entry
+  /** Advances the internal entry pointer
    *
-   * @return archive_entry or nullptr if end of archive reached
+   * @return true if the pointer advanced to the next entry
+   *         false if the end of the archive was reached
    */
-  ArchiveEntrySys& next();
+  bool advance();
 
-  /** Returns the current archive entry
+  /** Returns a non-owning view of the current entry
    *
-   * @return archive_entry or nullptr if current entry at end of archive
+   * ArchiveEntryView is a non-owning view of the currently
+   * active entry in this reader. A retrieved archive entry
+   * may not be used after another call to advance in the
+   * same reader.
+   *
+   * @return a view to the archive entry this reader currently
+   *         points to
    */
-  ArchiveEntrySys& cur();
+  const ArchiveEntryView cur();
 };
 
 /** A extractor for archive files
@@ -66,15 +64,13 @@ class ArchiveReaderSys {
 class ArchiveExtractorSys {
  private:
   archive* writer;
+
  public:
   ArchiveExtractorSys(std::filesystem::path& root);
   ArchiveExtractorSys();
 
-  void extract(ArchiveReaderSys& reader, ArchiveEntrySys& entry);
   void extract_all(ArchiveReaderSys& reader);
-
+  void extract_entry(ArchiveReaderSys& reader);
-  void extract_header(ArchiveEntrySys& entry);
-  void extract_data();
 };
 
 class ArchiveSysException : public std::exception {
@@ -89,9 +85,7 @@ class ArchiveSysException : public std::exception {
       _what = fmt::format("{}", what);
     }
   }
-  ArchiveSysException(std::string what) {
+  ArchiveSysException(std::string what) { _what = fmt::format("{}", what); }
-      _what = fmt::format("{}", what);
-  }
 
   virtual const char* what() const noexcept { return this->_what.c_str(); }
 };

src/main.cpp

@@ -5,13 +5,13 @@ namespace logger = spdlog;
 #include <iostream>
 #include <ostream>
 #include <string>
+#include <list>
 
 #include "archive.hpp"
 #include "spec.hpp"
 
 int main(int argc, char** argv) {
   logger::set_level(logger::level::trace);
-  logger::flush_on(logger::level::trace);
 
   try {
     std::filesystem::path filepath{argv[1]};