Skip to content

Commit

Permalink
Apply coderabbit suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
Bill-hbrhbr committed Nov 27, 2024
1 parent 89b5707 commit c646cea
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 41 deletions.
6 changes: 3 additions & 3 deletions components/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,13 @@ endif()
# TODO: add a script in ./cmake/Modules to resolve .a vs. .so
find_package(LibLZMA REQUIRED)
if(LIBLZMA_FOUND)
message(STATUS "Found LIBLZMA_FOUND ${LIBLZMA_VERSION_STRING}")
message(STATUS "Found Lzma ${LIBLZMA_VERSION_STRING}")
message(STATUS "Lzma library location: ${LIBLZMA_LIBRARIES}")
else()
message(FATAL_ERROR "Could not find ${CLP_LIBS_STRING} libraries for LIBLZMA_FOUND")
message(FATAL_ERROR "Could not find ${CLP_LIBS_STRING} libraries for Lzma")
endif()
include_directories(${LIBLZMA_INCLUDE_DIRS})
message("LZMA Include Dir: ${LIBLZMA_INCLUDE_DIRS}")
message("Lzma Include Dir: ${LIBLZMA_INCLUDE_DIRS}")

# sqlite dependencies
set(sqlite_DYNAMIC_LIBS "dl;m;pthread")
Expand Down
44 changes: 22 additions & 22 deletions components/core/src/clp/streaming_compression/lzma/Compressor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,13 @@ auto Compressor::open(FileWriter& file_writer, int compression_level) -> void {
throw OperationFailed(ErrorCode_Unsupported, __FILENAME__, __LINE__);
}

memset(m_compression_stream.get(), 0, sizeof(LzmaStream));
init_lzma_encoder(m_compression_stream.get(), compression_level, m_dict_size);
m_compression_stream = LZMA_STREAM_INIT;
init_lzma_encoder(&m_compression_stream, compression_level, m_dict_size);
// Setup compressed stream parameters
m_compression_stream->next_in = nullptr;
m_compression_stream->avail_in = 0;
m_compression_stream->next_out = m_compressed_stream_block_buffer.data();
m_compression_stream->avail_out = m_compressed_stream_block_buffer.size();
m_compression_stream.next_in = nullptr;
m_compression_stream.avail_in = 0;
m_compression_stream.next_out = m_compressed_stream_block_buffer.data();
m_compression_stream.avail_out = m_compressed_stream_block_buffer.size();

m_compressed_stream_file_writer = &file_writer;

Expand Down Expand Up @@ -119,14 +119,14 @@ auto Compressor::write(char const* data, size_t data_length) -> void {
throw OperationFailed(ErrorCode_BadParam, __FILENAME__, __LINE__);
}

m_compression_stream->next_in = size_checked_pointer_cast<uint8_t const>(data);
m_compression_stream->avail_in = data_length;
m_compression_stream.next_in = size_checked_pointer_cast<uint8_t const>(data);
m_compression_stream.avail_in = data_length;

// Normal compression encoding workflow. Continue until the input buffer is
// exhausted.
compress(LZMA_RUN);

m_compression_stream->next_in = nullptr;
m_compression_stream.next_in = nullptr;

m_compression_stream_contains_data = true;
m_uncompressed_stream_pos += data_length;
Expand Down Expand Up @@ -161,54 +161,54 @@ auto Compressor::flush_and_close_compression_stream() -> void {

m_compression_stream_contains_data = false;

lzma_end(m_compression_stream.get());
m_compression_stream->avail_out = 0;
m_compression_stream->next_out = nullptr;
lzma_end(&m_compression_stream);
m_compression_stream.avail_out = 0;
m_compression_stream.next_out = nullptr;
}

auto Compressor::compress(LzmaAction action) -> void {
bool hit_input_eof{false};
bool hit_stream_end{false};
while (true) {
auto const rc = lzma_code(m_compression_stream.get(), action);
auto const rc = lzma_code(&m_compression_stream, action);
switch (rc) {
case LZMA_OK:
case LZMA_BUF_ERROR:
break;
case LZMA_STREAM_END:
hit_input_eof = true;
hit_stream_end = true;
break;
default:
SPDLOG_ERROR("lzma() returned an unexpected value - {}.", static_cast<int>(rc));
throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__);
}

if (LZMA_RUN == action && 0 == m_compression_stream->avail_in) {
if (LZMA_RUN == action && 0 == m_compression_stream.avail_in) {
// No more data to compress
break;
}

if (hit_input_eof) {
if (hit_stream_end) {
break;
}

// Write output buffer to file if it's full
if (0 == m_compression_stream->avail_out) {
if (0 == m_compression_stream.avail_out) {
pipe_data();
}
}

// Write remaining compressed data
if (m_compression_stream->avail_out < cCompressedStreamBlockBufferSize) {
if (m_compression_stream.avail_out < cCompressedStreamBlockBufferSize) {
pipe_data();
}
}

auto Compressor::pipe_data() -> void {
m_compressed_stream_file_writer->write(
size_checked_pointer_cast<char>(m_compressed_stream_block_buffer.data()),
cCompressedStreamBlockBufferSize - m_compression_stream->avail_out
cCompressedStreamBlockBufferSize - m_compression_stream.avail_out
);
m_compression_stream->next_out = m_compressed_stream_block_buffer.data();
m_compression_stream->avail_out = cCompressedStreamBlockBufferSize;
m_compression_stream.next_out = m_compressed_stream_block_buffer.data();
m_compression_stream.avail_out = cCompressedStreamBlockBufferSize;
}
} // namespace clp::streaming_compression::lzma
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
#ifndef CLP_STREAMING_COMPRESSION_LZMA_COMPRESSOR_HPP
#define CLP_STREAMING_COMPRESSION_LZMA_COMPRESSOR_HPP

#include <zconf.h>

#include <cstddef>
#include <memory>

Expand Down Expand Up @@ -129,11 +127,11 @@ class Compressor : public ::clp::streaming_compression::Compressor {
FileWriter* m_compressed_stream_file_writer{nullptr};

// Compressed stream variables
std::unique_ptr<LzmaStream> m_compression_stream{std::make_unique<LzmaStream>()};
LzmaStream m_compression_stream;
bool m_compression_stream_contains_data{false};
size_t m_dict_size{cDefaultDictionarySize};

Array<Bytef> m_compressed_stream_block_buffer{cCompressedStreamBlockBufferSize};
Array<uint8_t> m_compressed_stream_block_buffer{cCompressedStreamBlockBufferSize};

size_t m_uncompressed_stream_pos{0};
};
Expand Down
24 changes: 12 additions & 12 deletions components/core/tools/scripts/lib_install/liblzma.sh
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
#!/bin/bash

# Exit on any error
set -e

# Error on undefined variable
set -u

# Dependencies:
# - curl
# - make
# - gcc
# NOTE: Dependencies should be installed outside the script to allow the script to be largely distro-agnostic

# Exit on any error
set -e

# Error on undefined variable
set -u
for cmd in curl make gcc; do
if ! $cmd --version >/dev/null 2>&1; then
echo "Error: Required dependency '$cmd' not found"
exit 1
fi
done

cUsage="Usage: ${BASH_SOURCE[0]} <version>[ <.deb output directory>]"
if [ "$#" -lt 1 ] ; then
Expand All @@ -32,13 +39,6 @@ fi

# Note: we won't check if the package already exists

echo "Checking for elevated privileges..."
privileged_command_prefix=""
if [ ${EUID:-$(id -u)} -ne 0 ] ; then
sudo echo "Script can elevate privileges."
privileged_command_prefix="${privileged_command_prefix} sudo"
fi

# Get number of cpu cores
num_cpus=$(grep -c ^processor /proc/cpuinfo)

Expand Down

0 comments on commit c646cea

Please sign in to comment.