Address review concern

Bill-hbrhbr · Dec 4, 2024 · 4164a9d · 4164a9d
1 parent eda7d6c
commit 4164a9d
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 33 deletions.
diff --git a/components/core/src/clp/streaming_compression/lzma/Compressor.cpp b/components/core/src/clp/streaming_compression/lzma/Compressor.cpp
@@ -144,12 +144,11 @@ auto Compressor::write(char const* data, size_t data_length) -> void {
     m_compression_stream.next_in = clp::size_checked_pointer_cast<uint8_t const>(data);
     m_compression_stream.avail_in = data_length;
 
-    while (m_compression_stream.avail_in > 0) {
-        encode_lzma_once();
-    }
+    encode_lzma();
 
     // All input data have been encoded so detach input data
     m_compression_stream.next_in = nullptr;
+    m_compression_stream.avail_in = 0;
 
     m_uncompressed_stream_pos += data_length;
 }
@@ -167,26 +166,31 @@ auto Compressor::try_get_pos(size_t& pos) const -> ErrorCode {
     return ErrorCode_Success;
 }
 
-auto Compressor::encode_lzma_once() -> void {
-    if (0 == m_compression_stream.avail_in) {
-        return;
-    }
+auto Compressor::encode_lzma() -> void {
+    while (m_compression_stream.avail_in > 0) {
+        // Write output buffer to file if it's full
+        if (0 == m_compression_stream.avail_out) {
+            flush_stream_output_block_buffer();
+        }
 
-    if (0 == m_compression_stream.avail_out) {
-        flush_stream_output_block_buffer();
+        auto const rc = lzma_code(&m_compression_stream, LZMA_RUN);
+        switch (rc) {
+            case LZMA_OK:
+                break;
+            case LZMA_BUF_ERROR:  // No encoding progress can be made
+                SPDLOG_ERROR("LZMA compressor input stream is corrupt.");
+                throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__);
+            default:
+                SPDLOG_ERROR(
+                        "lzma_code() returned an unexpected value - {}.",
+                        static_cast<int>(rc)
+                );
+                throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__);
+        }
     }
 
-    auto const rc = lzma_code(&m_compression_stream, LZMA_RUN);
-    switch (rc) {
-        case LZMA_OK:
-            break;
-        case LZMA_BUF_ERROR:  // No encoding progress can be made
-            SPDLOG_ERROR("LZMA compressor input stream is corrupt.");
-            throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__);
-        default:
-            SPDLOG_ERROR("lzma_code() returned an unexpected value - {}.", static_cast<int>(rc));
-            throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__);
-    }
+    // Write the last chunk of output
+    flush_stream_output_block_buffer();
 }
 
 auto Compressor::flush_lzma(lzma_action flush_action) -> void {
@@ -198,8 +202,18 @@ auto Compressor::flush_lzma(lzma_action flush_action) -> void {
         throw OperationFailed(ErrorCode_BadParam, __FILENAME__, __LINE__);
     }
 
+    /**
+     * Once flushing starts, the workflow action needs to stay the same until
+     * flushing is signaled completed by LZMA (aka LZMA_STREAM_END is reached).
+     * See also: https://github.com/tukaani-project/xz/blob/master/src/liblzma/api/lzma/base.h#L274
+     */
     bool flushed{false};
     while (false == flushed) {
+        // Write output buffer to file if it's full
+        if (0 == m_compression_stream.avail_out) {
+            flush_stream_output_block_buffer();
+        }
+
         auto const rc = lzma_code(&m_compression_stream, flush_action);
         switch (rc) {
             case LZMA_OK:
@@ -222,11 +236,6 @@ auto Compressor::flush_lzma(lzma_action flush_action) -> void {
                 );
                 throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__);
         }
-
-        // Write output buffer to file if it's full
-        if (0 == m_compression_stream.avail_out) {
-            flush_stream_output_block_buffer();
-        }
     }
 
     // Write the last chunk of output

diff --git a/components/core/src/clp/streaming_compression/lzma/Compressor.hpp b/components/core/src/clp/streaming_compression/lzma/Compressor.hpp
@@ -91,22 +91,24 @@ class Compressor : public ::clp::streaming_compression::Compressor {
     static constexpr size_t cCompressedStreamBlockBufferSize{4096};  // 4KiB
 
     /**
-     * Invoke lzma_code() encoding workflow once with LZMA_RUN
+     * Invoke lzma_code() repeatedly with LZMA_RUN until the input is exhausted
      *
-     * The encoded data may be buffered and thus not immediately available at
-     * the output block.
+     * At the end of the workflow, the last bytes of encoded data may still be
+     * buffered and thus not immediately available at the output block buffer.
+     *
+     * Assumes input stream and output block buffer are both in valid states.
+     * @throw `OperationFailed` if LZMA returns an unexpected error value
      */
-    auto encode_lzma_once() -> void;
+    auto encode_lzma() -> void;
 
     /**
      * Invoke lzma_code() repeatedly with the given flushing action until all
      * encoded data is made available at the output block buffer
      *
-     * Once flushing starts, the workflow action needs to stay the same until
-     * flushing is signaled completed by LZMA (aka LZMA_STREAM_END is reached).
-     * See also: https://github.com/tukaani-project/xz/blob/master/src/liblzma/api/lzma/base.h#L274
-     *
+     * Assumes input stream and output block buffer are both in valid states.
      * @param flush_action
+     * @throw `OperationFailed` if the provided action is not an LZMA flush
+     *        action, or if LZMA returns an unexpected error value
      */
     auto flush_lzma(lzma_action flush_action) -> void;
 

diff --git a/components/core/tests/test-StreamingCompression.cpp b/components/core/tests/test-StreamingCompression.cpp
@@ -16,6 +16,7 @@
 #include "../src/clp/FileWriter.hpp"
 #include "../src/clp/ReadOnlyMemoryMappedFile.hpp"
 #include "../src/clp/streaming_compression/Compressor.hpp"
+#include "../src/clp/streaming_compression/Decompressor.hpp"
 #include "../src/clp/streaming_compression/lzma/Compressor.hpp"
 #include "../src/clp/streaming_compression/passthrough/Compressor.hpp"
 #include "../src/clp/streaming_compression/passthrough/Decompressor.hpp"
@@ -26,6 +27,7 @@ using clp::Array;
 using clp::ErrorCode_Success;
 using clp::FileWriter;
 using clp::streaming_compression::Compressor;
+using clp::streaming_compression::Decompressor;
 using std::string;
 using std::string_view;