Address review comments

components/core/src/clp_s/ArchiveReaderAdaptor.cpp

@@ -35,10 +35,6 @@ ArchiveReaderAdaptor::ArchiveReaderAdaptor(
     }
 }
 
-ArchiveReaderAdaptor::~ArchiveReaderAdaptor() {
-    m_reader.reset();
-}
-
 ErrorCode
 ArchiveReaderAdaptor::try_read_archive_file_info(ZstdDecompressor& decompressor, size_t size) {
     std::vector<char> buffer(size);
@@ -190,6 +186,21 @@ ErrorCode ArchiveReaderAdaptor::try_read_archive_metadata(ZstdDecompressor& deco
     return ErrorCodeSuccess;
 }
 
+std::shared_ptr<clp::ReaderInterface> ArchiveReaderAdaptor::try_create_reader_at_header() {
+    if (InputSource::Filesystem == m_archive_path.source && false == m_single_file_archive) {
+        try {
+            return std::make_shared<clp::FileReader>(
+                    m_archive_path.path + constants::cArchiveHeaderFile
+            );
+        } catch (std::exception const& e) {
+            SPDLOG_ERROR("Failed to open archive header for reading - {}", e.what());
+            return nullptr;
+        }
+    } else {
+        return ReaderUtils::try_create_reader(m_archive_path, m_network_auth);
+    }
+}
+
 std::unique_ptr<clp::ReaderInterface> ArchiveReaderAdaptor::checkout_reader_for_section(
         std::string_view section
 ) {
@@ -255,19 +266,4 @@ void ArchiveReaderAdaptor::checkin_reader_for_section(std::string_view section)
 
     m_current_reader_holder.reset();
 }
-
-std::shared_ptr<clp::ReaderInterface> ArchiveReaderAdaptor::try_create_reader_at_header() {
-    if (InputSource::Filesystem == m_archive_path.source && false == m_single_file_archive) {
-        try {
-            return std::make_shared<clp::FileReader>(
-                    m_archive_path.path + constants::cArchiveHeaderFile
-            );
-        } catch (std::exception const& e) {
-            SPDLOG_ERROR("Failed to open archive header for reading - {}", e.what());
-            return nullptr;
-        }
-    } else {
-        return ReaderUtils::try_create_reader(m_archive_path, m_network_auth);
-    }
-}
 }  // namespace clp_s

components/core/src/clp_s/ArchiveReaderAdaptor.hpp

@@ -30,19 +30,17 @@ class ArchiveReaderAdaptor {
 
     explicit ArchiveReaderAdaptor(Path const& archive_path, NetworkAuthOption const& network_auth);
 
-    ~ArchiveReaderAdaptor();
-
     /**
      * Loads metadata for an archive including the header and metadata section. This method must be
      * invoked before checking out any section of an archive, or calling `get_timestamp_dictionary`.
-     * @return ErrorCodeSuccess on success
-     * @return ErrorCode_errno on failure
+     * @return ErrorCodeSuccess on success.
+     * @return relevant ErrorCode on failure.
      */
     ErrorCode load_archive_metadata();
 
     /**
-     * Checks out a reader for a given section of the archive. Reader must be checked back in with the
-     * `checkin_reader_for_section` method.
+     * Checks out a reader for a given section of the archive. Reader must be checked back in with
+     * the `checkin_reader_for_section` method.
      * @param section
      * @return A ReaderInterface opened and pointing to the requested section.
      * @throw OperationFailed if a reader is already checked out, or checking out this section would
@@ -65,19 +63,64 @@ class ArchiveReaderAdaptor {
     ArchiveHeader const& get_header() const { return m_archive_header; }
 
 private:
+    /**
+     * Tries to read an ArchiveFileInfo packet from the archive metadata.
+     * @param decompressor
+     * @param size The number of decompressed bytes making up the packet.
+     * @return ErrorCodeSuccess on success.
+     * @return relevant ErrorCode on failure.
+     */
     ErrorCode try_read_archive_file_info(ZstdDecompressor& decompressor, size_t size);
 
+    /**
+     * Tries to read an TimestampDictionary packet from the archive metadata.
+     * @param decompressor
+     * @param size The number of decompressed bytes making up the packet.
+     * @return ErrorCodeSuccess on success.
+     * @return relevant ErrorCode on failure.
+     */
     ErrorCode try_read_timestamp_dictionary(ZstdDecompressor& decompressor, size_t size);
 
+    /**
+     * Tries to read an ArchiveInfo packet from the archive metadata.
+     * @param decompressor
+     * @param size The number of decompressed bytes making up the packet.
+     * @return ErrorCodeSuccess on success.
+     * @return relevant ErrorCode on failure.
+     */
     ErrorCode try_read_archive_info(ZstdDecompressor& decompressor, size_t size);
 
+    /**
+     * Tries to create a reader for the archive header.
+     * @return A ReaderInterface opened and pointing to the archive header on success.
+     * @return nullptr on failure.
+     */
     std::shared_ptr<clp::ReaderInterface> try_create_reader_at_header();
 
+    /**
+     * Checks out a reader for a given section of the single file archive.
+     * @param section
+     * @return A ReaderInterface opened and pointing to the requested section.
+     * @throw OperationFailed if the requested section does not exist in ArchiveFileInfo, if
+     *        checking out the section would force a backward seek, or on any I/O error.
+     */
     std::unique_ptr<clp::ReaderInterface> checkout_reader_for_sfa_section(std::string_view section);
 
+    /**
+     * Tries to read the header for the archive from the given reader.
+     * @param reader
+     * @return ErrorCodeSuccess on success.
+     * @return relevant ErrorCode on failure.
+     */
     ErrorCode try_read_header(clp::ReaderInterface& reader);
 
-    ErrorCode try_read_archive_metadata(ZstdDecompressor& reader);
+    /**
+     * Tries to read the archive metadata from the given decompressor.
+     * @param decompressor
+     * @return ErrorCodeSuccess on success.
+     * @return relevant ErrorCode on failure.
+     */
+    ErrorCode try_read_archive_metadata(ZstdDecompressor& decompressor);
 
     Path m_archive_path{};
     NetworkAuthOption m_network_auth{};

components/core/src/clp_s/TimestampDictionaryReader.cpp

@@ -42,7 +42,7 @@ ErrorCode TimestampDictionaryReader::read(ZstdDecompressor& decompressor) {
     if (ErrorCodeSuccess != error) {
         return error;
     }
-    for (int i = 0; i < num_patterns; ++i) {
+    for (uint64_t i = 0; i < num_patterns; ++i) {
         uint64_t id, pattern_len;
         std::string pattern;
         error = decompressor.try_read_numeric_value<uint64_t>(id);

components/core/src/clp_s/ZstdDecompressor.cpp

@@ -287,12 +287,13 @@ ErrorCode ZstdDecompressor::open(std::string const& compressed_file_path) {
 
 void ZstdDecompressor::reset_stream() {
     if (InputType::File == m_input_type) {
-        auto rc = m_file_reader->try_seek_from_begin(m_file_reader_initial_pos);
-        m_file_read_buffer_length = 0;
-        m_compressed_stream_block.size = m_file_read_buffer_length;
-        if (false == (ErrorCodeSuccess == rc || ErrorCodeEndOfFile == rc)) {
+        if (auto rc = m_file_reader->try_seek_from_begin(m_file_reader_initial_pos);
+            ErrorCodeSuccess != rc && ErrorCodeEndOfFile != rc)
+        {
             throw OperationFailed(rc, __FILENAME__, __LINE__);
         }
+        m_file_read_buffer_length = 0;
+        m_compressed_stream_block.size = m_file_read_buffer_length;
     } else if (InputType::ClpReader == m_input_type) {
         auto rc = m_reader->try_seek_from_begin(m_file_reader_initial_pos);
         m_file_read_buffer_length = 0;