From 203731c343f70d5deb9b37a8537cef6cf98ee43f Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Sun, 6 Oct 2024 16:13:14 -0400 Subject: [PATCH] D'oh, ban copy/move constructors 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. --- src/helpers.cpp | 15 +++++++++++++-- test/helpers.test.cpp | 19 +++++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/helpers.cpp b/src/helpers.cpp index 6397d7c4..751bd7ce 100644 --- a/src/helpers.cpp +++ b/src/helpers.cpp @@ -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); } @@ -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); } @@ -88,7 +100,6 @@ std::vector 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 ? std::string compress_string(const std::string& str, int compressionlevel, bool asGzip) { @@ -96,7 +107,7 @@ std::string compress_string(const std::string& str, compressionlevel = 6; if (compressionlevel != compressor.level) - compressor = Compressor(compressionlevel); + compressor.setLevel(compressionlevel); std::string rv; if (asGzip) { diff --git a/test/helpers.test.cpp b/test/helpers.test.cpp index c9449aed..c5e2be49 100644 --- a/test/helpers.test.cpp +++ b/test/helpers.test.cpp @@ -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);