Skip to content

Commit

Permalink
D'oh, ban copy/move constructors
Browse files Browse the repository at this point in the history
Oops: if you ever passed a non-default compression level, we'd
try to create a new compressor. But because we were using the
default constructors, we'd happily create a temporary, copy its
pointer over to the `compressor` field, then destruct the temporary,
freeing the underlying native resource, and the world would end.

Instead, delete the default constructors and require an explicit
`setLevel` call.

Also update tests to verify that the different levels work.
  • Loading branch information
cldellow committed Oct 6, 2024
1 parent d8975d3 commit 203731c
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 4 deletions.
15 changes: 13 additions & 2 deletions src/helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,21 @@ class Compressor {
libdeflate_compressor* compressor;

Compressor(int level): level(level), compressor(NULL) {
setLevel(level);
}

void setLevel(int level) {
libdeflate_free_compressor(compressor);
this->level = level;
compressor = libdeflate_alloc_compressor(level);

if (!compressor)
throw std::runtime_error("libdeflate_alloc_compressor failed (level=" + std::to_string(level) + ")");
}

Compressor & operator=(const Compressor&) = delete;
Compressor(const Compressor&) = delete;

~Compressor() {
libdeflate_free_compressor(compressor);
}
Expand All @@ -54,6 +63,9 @@ class Decompressor {
throw std::runtime_error("libdeflate_alloc_decompressor failed");
}

Decompressor & operator=(const Decompressor&) = delete;
Decompressor(const Decompressor&) = delete;

~Decompressor() {
libdeflate_free_decompressor(decompressor);
}
Expand Down Expand Up @@ -88,15 +100,14 @@ std::vector<std::string> parseBox(const std::string& bbox) {
}

// Compress a STL string using zlib with given compression level, and return the binary data
// TODO: consider returning a std::vector<char> ?
std::string compress_string(const std::string& str,
int compressionlevel,
bool asGzip) {
if (compressionlevel == Z_DEFAULT_COMPRESSION)
compressionlevel = 6;

if (compressionlevel != compressor.level)
compressor = Compressor(compressionlevel);
compressor.setLevel(compressionlevel);

std::string rv;
if (asGzip) {
Expand Down
19 changes: 17 additions & 2 deletions test/helpers.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,31 @@ MU_TEST(test_get_chunks) {
MU_TEST(test_compression_gzip) {
std::string input = "a random string to be compressed";

std::string gzipped = compress_string(input, 6, true);
for (int level = 1; level < 9; level++) {
std::string gzipped = compress_string(input, level, true);
std::string unzipped;
decompress_string(unzipped, gzipped.data(), gzipped.size(), true);
mu_check(unzipped == input);
}

std::string gzipped = compress_string(input, -1, true);
std::string unzipped;
decompress_string(unzipped, gzipped.data(), gzipped.size(), true);
mu_check(unzipped == input);

}

MU_TEST(test_compression_zlib) {
std::string input = "a random string to be compressed";

std::string gzipped = compress_string(input, 6, false);
for (int level = 1; level < 9; level++) {
std::string gzipped = compress_string(input, level, false);
std::string unzipped;
decompress_string(unzipped, gzipped.data(), gzipped.size(), false);
mu_check(unzipped == input);
}

std::string gzipped = compress_string(input, -1, false);
std::string unzipped;
decompress_string(unzipped, gzipped.data(), gzipped.size(), false);
mu_check(unzipped == input);
Expand Down

0 comments on commit 203731c

Please sign in to comment.