From 8249a2f0172fa4fde39d231360a267816e3a802c Mon Sep 17 00:00:00 2001 From: Armin Friedl Date: Sat, 22 Feb 2020 20:13:32 +0100 Subject: [PATCH] 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 | 21 +++++++------ src/archive_sys.cpp | 75 ++++++++++++++++++++++----------------------- src/archive_sys.hpp | 52 ++++++++++++++----------------- src/main.cpp | 2 +- 4 files changed, 72 insertions(+), 78 deletions(-) diff --git a/src/archive.cpp b/src/archive.cpp index 09427df..cbf2497 100644 --- a/src/archive.cpp +++ b/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) { - ArchiveEntrySys& entry = archive_reader.next(); - if(entry.is_empty()) break; + while (archive_reader.advance()) { + ArchiveEntryView entry = archive_reader.cur(); 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 (first_entry.is_empty()) { // archive is empty + if (!archive_reader.advance()) { // can't advance even once, 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); } diff --git a/src/archive_sys.cpp b/src/archive_sys.cpp index f262b8b..b924d76 100644 --- a/src/archive_sys.cpp +++ b/src/archive_sys.cpp @@ -8,23 +8,27 @@ namespace logger = spdlog; #include #include -bool xwim::ArchiveEntrySys::is_empty() { - return (this->ae.get() == nullptr); +bool xwim::ArchiveEntryView::is_empty() { + return (this->ae == nullptr); } -std::string xwim::ArchiveEntrySys::path_name() { - return archive_entry_pathname(this->ae.get()); +std::string xwim::ArchiveEntryView::path_name() { + 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() { - return archive_entry_filetype(this->ae.get()); +mode_t xwim::ArchiveEntryView::file_type() { + 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"); - archive_entry* ae; - r = archive_read_next_header(this->ar, &ae); - if (r != ARCHIVE_OK) - throw(ArchiveSysException{"Could not list archive", this->ar}); + logger::trace("Advancing reader to next archive entry"); - 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 archive header"); - - return this->cur_entry; + logger::trace("Got entry {}", archive_entry_pathname(ae)); + return true; } -xwim::ArchiveEntrySys& xwim::ArchiveReaderSys::cur() { - return this->cur_entry; +const xwim::ArchiveEntryView xwim::ArchiveReaderSys::cur() { + 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(;;) { - ArchiveEntrySys& entry = reader.next(); + while(reader.advance()) { + 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)); - } -} diff --git a/src/archive_sys.hpp b/src/archive_sys.hpp index de9c63a..9225c80 100644 --- a/src/archive_sys.hpp +++ b/src/archive_sys.hpp @@ -7,22 +7,13 @@ namespace xwim { -class ArchiveEntrySys { +class ArchiveEntryView { private: - std::function ae_deleter = [](archive_entry* ae) { archive_entry_free(ae); }; - std::unique_ptr ae; - friend class ArchiveExtractorSys; + archive_entry* ae; public: - ArchiveEntrySys(std::unique_ptr entry) - : ae{ std::unique_ptr{entry.release(), ae_deleter} } - {} - ArchiveEntrySys() - : ae{ std::unique_ptr{nullptr, ae_deleter} } - {} - ArchiveEntrySys(archive_entry* entry) - : ae{ std::unique_ptr{entry, ae_deleter} } - {} + ArchiveEntryView() = default; + ArchiveEntryView(archive_entry* entry) : ae{entry} {} 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 @@ -64,17 +62,15 @@ class ArchiveReaderSys { * Shim for `libarchive`. */ class ArchiveExtractorSys { -private: + private: archive* writer; -public: + + public: ArchiveExtractorSys(std::filesystem::path& root); ArchiveExtractorSys(); - void extract(ArchiveReaderSys& reader, ArchiveEntrySys& entry); void extract_all(ArchiveReaderSys& reader); - - void extract_header(ArchiveEntrySys& entry); - void extract_data(); + void extract_entry(ArchiveReaderSys& reader); }; class ArchiveSysException : public std::exception { @@ -89,9 +85,7 @@ class ArchiveSysException : public std::exception { _what = fmt::format("{}", what); } } - ArchiveSysException(std::string what) { - _what = fmt::format("{}", what); - } + ArchiveSysException(std::string what) { _what = fmt::format("{}", what); } virtual const char* what() const noexcept { return this->_what.c_str(); } }; diff --git a/src/main.cpp b/src/main.cpp index bcf5bc9..c34037a 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -5,13 +5,13 @@ namespace logger = spdlog; #include #include #include +#include #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]};