diff --git a/src/libslic3r/PrintExport.hpp b/src/libslic3r/PrintExport.hpp index 1b5efcea1..cdb2cc008 100644 --- a/src/libslic3r/PrintExport.hpp +++ b/src/libslic3r/PrintExport.hpp @@ -80,9 +80,8 @@ public: // Provokes static_assert in the right way. template struct VeryFalse { static const bool value = false; }; -// This has to be explicitly implemented in the gui layer or a default zlib -// based implementation is needed. I don't have time for that and I'm delegating -// the implementation to the gui layer where the gui toolkit can cover this. +// This can be explicitly implemented in the gui layer or the default Zipper +// API in libslic3r with minz. template class LayerWriter { public: @@ -91,21 +90,28 @@ public: "No layer writer implementation provided!"); } + // Should create a new file within the zip with the given filename. It + // should also finish any previous entry. void next_entry(const std::string& /*fname*/) {} - // binary entry + // Should create a new file within the archive and write the provided data. void binary_entry(const std::string& /*fname*/, const std::uint8_t* buf, size_t len); + // Get the name of the archive but only the name part without the path or + // the extension. std::string get_name() { return ""; } + // Test whether the object can still be used for writing. bool is_ok() { return false; } + // Write some data (text) into the current file (entry) within the archive. template LayerWriter& operator<<(T&& /*arg*/) { return *this; } - void close() {} + // Flush the current entry into the archive. + void finalize() {} }; // Implementation for PNG raster output @@ -267,7 +273,7 @@ public: } } - writer.close(); + writer.finalize(); } catch(std::exception& e) { BOOST_LOG_TRIVIAL(error) << e.what(); // Rethrow the exception diff --git a/src/libslic3r/SLAPrint.hpp b/src/libslic3r/SLAPrint.hpp index 454f5870f..d831908c7 100644 --- a/src/libslic3r/SLAPrint.hpp +++ b/src/libslic3r/SLAPrint.hpp @@ -322,18 +322,18 @@ template<> class LayerWriter { Zipper m_zip; public: - inline LayerWriter(const std::string& zipfile_path): m_zip(zipfile_path) {} + LayerWriter(const std::string& zipfile_path): m_zip(zipfile_path) {} - inline void next_entry(const std::string& fname) { m_zip.add_entry(fname); } + void next_entry(const std::string& fname) { m_zip.add_entry(fname); } - inline void binary_entry(const std::string& fname, + void binary_entry(const std::string& fname, const std::uint8_t* buf, size_t l) { m_zip.add_entry(fname, buf, l); } - inline std::string get_name() const { + std::string get_name() const { return m_zip.get_name(); } @@ -345,7 +345,11 @@ public: return true; // m_zip blows up if something goes wrong... } - inline void close() { m_zip.close(); } + // After finalize, no writing to the archive will have an effect. The only + // valid operation is to dispose the object calling the destructor which + // should close the file. This method can throw and signal potential errors + // when flushing the archive. This is why its present. + void finalize() { m_zip.finalize(); } }; /** diff --git a/src/libslic3r/Zipper.cpp b/src/libslic3r/Zipper.cpp index 490805c0d..6b7faaddc 100644 --- a/src/libslic3r/Zipper.cpp +++ b/src/libslic3r/Zipper.cpp @@ -111,6 +111,11 @@ public: { throw std::runtime_error(formatted_errorstr()); } + + bool is_alive() + { + return arch.m_zip_mode != MZ_ZIP_MODE_WRITING_HAS_BEEN_FINALIZED; + } }; Zipper::Zipper(const std::string &zipfname, e_compression compression) @@ -129,11 +134,19 @@ Zipper::Zipper(const std::string &zipfname, e_compression compression) Zipper::~Zipper() { - try { - close(); - } catch(...) { - BOOST_LOG_TRIVIAL(error) << m_impl->formatted_errorstr(); + if(m_impl->is_alive()) { + // Flush the current entry if not finished yet. + try { finish_entry(); } catch(...) { + BOOST_LOG_TRIVIAL(error) << m_impl->formatted_errorstr(); + } + + if(!mz_zip_writer_finalize_archive(&m_impl->arch)) + BOOST_LOG_TRIVIAL(error) << m_impl->formatted_errorstr(); } + + // The file should be closed no matter what... + if(!mz_zip_writer_end(&m_impl->arch)) + BOOST_LOG_TRIVIAL(error) << m_impl->formatted_errorstr(); } Zipper::Zipper(Zipper &&m): @@ -152,12 +165,16 @@ Zipper &Zipper::operator=(Zipper &&m) { void Zipper::add_entry(const std::string &name) { + if(!m_impl->is_alive()) return; + finish_entry(); // finish previous business m_entry = name; } void Zipper::add_entry(const std::string &name, const uint8_t *data, size_t l) { + if(!m_impl->is_alive()) return; + finish_entry(); mz_uint cmpr = MZ_NO_COMPRESSION; switch (m_compression) { @@ -175,7 +192,9 @@ void Zipper::add_entry(const std::string &name, const uint8_t *data, size_t l) void Zipper::finish_entry() { - if(!m_data.empty() > 0 && !m_entry.empty()) { + if(!m_impl->is_alive()) return; + + if(!m_data.empty() && !m_entry.empty()) { mz_uint compression = MZ_NO_COMPRESSION; switch (m_compression) { @@ -198,12 +217,12 @@ std::string Zipper::get_name() const { return boost::filesystem::path(m_impl->m_zipname).stem().string(); } -void Zipper::close() +void Zipper::finalize() { finish_entry(); - if(!mz_zip_writer_finalize_archive(&m_impl->arch)) m_impl->blow_up(); - if(!mz_zip_writer_end(&m_impl->arch)) m_impl->blow_up(); + if(m_impl->is_alive()) if(!mz_zip_writer_finalize_archive(&m_impl->arch)) + m_impl->blow_up(); } } diff --git a/src/libslic3r/Zipper.hpp b/src/libslic3r/Zipper.hpp index 7319c4ac4..6566dad42 100644 --- a/src/libslic3r/Zipper.hpp +++ b/src/libslic3r/Zipper.hpp @@ -47,6 +47,7 @@ public: void add_entry(const std::string& name); /// Add a new binary file entry with an instantly given byte buffer. + /// This method throws exactly like finish_entry() does. void add_entry(const std::string& name, const std::uint8_t* data, size_t l); // Writing data to the archive works like with standard streams. The target @@ -74,12 +75,16 @@ public: /// buffer and ones an entry is added, the buffer will bind to the new entry /// If the buffer was written, but no entry was added, the buffer will be /// cleared after this call. + /// + /// This method will throw a runtime exception if an error occures. The + /// entry will still be open (with the data intact) but the state of the + /// file is up to minz after the erroneous write. void finish_entry(); /// Gets the name of the archive without the path or extension. std::string get_name() const; - void close(); + void finalize(); };