Repository: randombit/botan
Pull request #2684 (2021-04-03 14:47:50)

Remove use of explicit new from compression APIs
GH #2677 


Patch:
diff --git a/doc/api_ref/compression.rst b/doc/api_ref/compression.rst
index 5637a5a68..56a75ad9d 100644
--- a/doc/api_ref/compression.rst
+++ b/doc/api_ref/compression.rst
@@ -73,13 +73,20 @@ finally completing processing the stream (``finish``).
        This function may throw if the data seems to be invalid.
 
 The easiest way to get a compressor is via the functions
+``Compression_Algorithm::create`` and
+``Decompression_Algorithm::create`` which both accept a string
+argument which can take values include `zlib` (raw zlib with no
+checksum), `deflate` (zlib's deflate format), `gzip`, `bz2`, and
+`lzma`. A null pointer will be returned if the algorithm is
+unavailable.
+
+Two older functions for this are
 
 .. cpp:function:: Compression_Algorithm* make_compressor(std::string type)
 .. cpp:function:: Decompression_Algorithm* make_decompressor(std::string type)
 
-Supported values for `type` include `zlib` (raw zlib with no checksum),
-`deflate` (zlib's deflate format), `gzip`, `bz2`, and `lzma`. A null pointer
-will be returned if the algorithm is unavailable.
+which call the relevant ``create`` function and then ``release`` the
+returned ``unique_ptr``. Avoid these in new code.
 
 To use a compression algorithm in a `Pipe` use the adapter types
 `Compression_Filter` and `Decompression_Filter` from `comp_filter.h`. The
diff --git a/src/build-data/arch/riscv32.txt b/src/build-data/arch/riscv32.txt
index c8c258a00..ba88b42e9 100644
--- a/src/build-data/arch/riscv32.txt
+++ b/src/build-data/arch/riscv32.txt
@@ -1,2 +1,2 @@
-family riscv
+family riscv32
 endian little
diff --git a/src/cli/compress.cpp b/src/cli/compress.cpp
index e62acd763..b52e15608 100644
--- a/src/cli/compress.cpp
+++ b/src/cli/compress.cpp
@@ -55,10 +55,7 @@ class Compress final : public Command
          const size_t buf_size = get_arg_sz("buf-size");
          const size_t comp_level = get_arg_sz("level");
 
-         std::unique_ptr<Botan::Compression_Algorithm> compress;
-
-         compress.reset(Botan::make_compressor(comp_type));
-
+         auto compress = Botan::Compression_Algorithm::create(comp_type);
          if(!compress)
             {
             throw CLI_Error_Unsupported("Compression", comp_type);
@@ -146,9 +143,7 @@ class Decompress final : public Command
             throw CLI_IO_Error("reading", in_file);
             }
 
-         std::unique_ptr<Botan::Decompression_Algorithm> decompress;
-
-         decompress.reset(Botan::make_decompressor(suffix));
+         auto decompress = Botan::Decompression_Algorithm::create(suffix);
 
          if(!decompress)
             {
diff --git a/src/cli/entropy.cpp b/src/cli/entropy.cpp
index 0404afb99..472765704 100644
--- a/src/cli/entropy.cpp
+++ b/src/cli/entropy.cpp
@@ -63,7 +63,7 @@ class Entropy final : public Command
 #if defined(BOTAN_HAS_COMPRESSION)
             if(!sample.empty())
                {
-               std::unique_ptr<Botan::Compression_Algorithm> comp(Botan::make_compressor("zlib"));
+               auto comp = Botan::Compression_Algorithm::create("zlib");
                if(comp)
                   {
                   try
diff --git a/src/cli/speed.cpp b/src/cli/speed.cpp
index 4a22fb57f..6bd0405ef 100644
--- a/src/cli/speed.cpp
+++ b/src/cli/speed.cpp
@@ -1085,7 +1085,7 @@ class Speed final : public Command
             size_t compressed_size = 0;
 
 #if defined(BOTAN_HAS_ZLIB)
-            std::unique_ptr<Botan::Compression_Algorithm> comp(Botan::make_compressor("zlib"));
+            auto comp = Botan::Compression_Algorithm::create("zlib");
 
             if(comp)
                {
diff --git a/src/lib/compression/bzip2/bzip2.cpp b/src/lib/compression/bzip2/bzip2.cpp
index df6ad562f..15bdbfb18 100644
--- a/src/lib/compression/bzip2/bzip2.cpp
+++ b/src/lib/compression/bzip2/bzip2.cpp
@@ -97,14 +97,14 @@ class Bzip2_Decompression_Stream final : public Bzip2_Stream
 
 }
 
-Compression_Stream* Bzip2_Compression::make_stream(size_t comp_level) const
+std::unique_ptr<Compression_Stream> Bzip2_Compression::make_stream(size_t comp_level) const
    {
-   return new Bzip2_Compression_Stream(comp_level);
+   return std::make_unique<Bzip2_Compression_Stream>(comp_level);
    }
 
-Compression_Stream* Bzip2_Decompression::make_stream() const
+std::unique_ptr<Compression_Stream> Bzip2_Decompression::make_stream() const
    {
-   return new Bzip2_Decompression_Stream;
+   return std::make_unique<Bzip2_Decompression_Stream>();
    }
 
 }
diff --git a/src/lib/compression/bzip2/bzip2.h b/src/lib/compression/bzip2/bzip2.h
index e056d5502..e199cf154 100644
--- a/src/lib/compression/bzip2/bzip2.h
+++ b/src/lib/compression/bzip2/bzip2.h
@@ -21,7 +21,7 @@ class BOTAN_PUBLIC_API(2,0) Bzip2_Compression final : public Stream_Compression
    public:
       std::string name() const override { return "Bzip2_Compression"; }
    private:
-      Compression_Stream* make_stream(size_t comp_level) const override;
+      std::unique_ptr<Compression_Stream> make_stream(size_t comp_level) const override;
    };
 
 /**
@@ -32,7 +32,7 @@ class BOTAN_PUBLIC_API(2,0) Bzip2_Decompression final : public Stream_Decompress
    public:
       std::string name() const override { return "Bzip2_Decompression"; }
    private:
-      Compression_Stream* make_stream() const override;
+      std::unique_ptr<Compression_Stream> make_stream() const override;
    };
 
 }
diff --git a/src/lib/compression/compress_utils.cpp b/src/lib/compression/compress_utils.cpp
index f49a0ede1..d7df331c6 100644
--- a/src/lib/compression/compress_utils.cpp
+++ b/src/lib/compression/compress_utils.cpp
@@ -54,7 +54,7 @@ void Stream_Compression::clear()
 
 void Stream_Compression::start(size_t level)
    {
-   m_stream.reset(make_stream(level));
+   m_stream = make_stream(level);
    }
 
 void Stream_Compression::process(secure_vector<uint8_t>& buf, size_t offset, uint32_t flags)
@@ -129,7 +129,7 @@ void Stream_Decompression::clear()
 
 void Stream_Decompression::start()
    {
-   m_stream.reset(make_stream());
+   m_stream = make_stream();
    }
 
 void Stream_Decompression::process(secure_vector<uint8_t>& buf, size_t offset, uint32_t flags)
diff --git a/src/lib/compression/compression.cpp b/src/lib/compression/compression.cpp
index 361bf7dd3..f1192fd5b 100644
--- a/src/lib/compression/compression.cpp
+++ b/src/lib/compression/compression.cpp
@@ -24,39 +24,33 @@
 
 namespace Botan {
 
-Compression_Algorithm* make_compressor(const std::string& name)
+//static
+std::unique_ptr<Compression_Algorithm>
+Compression_Algorithm::create(const std::string& name)
    {
 #if defined(BOTAN_HAS_ZLIB)
    if(name == "Zlib" || name == "zlib")
-      return new Zlib_Compression;
+      return std::make_unique<Zlib_Compression>();
    if(name == "Gzip" || name == "gzip" || name == "gz")
-      return new Gzip_Compression;
+      return std::make_unique<Gzip_Compression>();
    if(name == "Deflate" || name == "deflate")
-      return new Deflate_Compression;
+      return std::make_unique<Deflate_Compression>();
 #endif
 
 #if defined(BOTAN_HAS_BZIP2)
    if(name == "bzip2" || name == "bz2" || name == "Bzip2")
-      return new Bzip2_Compression;
+      return std::make_unique<Bzip2_Compression>();
 #endif
 
 #if defined(BOTAN_HAS_LZMA)
    if(name == "lzma" || name == "xz" || name == "LZMA")
-      return new LZMA_Compression;
+      return std::make_unique<LZMA_Compression>();
 #endif
 
    BOTAN_UNUSED(name);
    return nullptr;
    }
 
-//static
-std::unique_ptr<Compression_Algorithm>
-Compression_Algorithm::create(const std::string& algo)
-   {
-   std::unique_ptr<Compression_Algorithm> compressor(make_compressor(algo));
-   return compressor;
-   }
-
 //static
 std::unique_ptr<Compression_Algorithm>
 Compression_Algorithm::create_or_throw(const std::string& algo)
@@ -68,39 +62,33 @@ Compression_Algorithm::create_or_throw(const std::string& algo)
    throw Lookup_Error("Compression", algo, "");
    }
 
-Decompression_Algorithm* make_decompressor(const std::string& name)
+//static
+std::unique_ptr<Decompression_Algorithm>
+Decompression_Algorithm::create(const std::string& name)
    {
 #if defined(BOTAN_HAS_ZLIB)
    if(name == "Zlib" || name == "zlib")
-      return new Zlib_Decompression;
+      return std::make_unique<Zlib_Decompression>();
    if(name == "Gzip" || name == "gzip" || name == "gz")
-      return new Gzip_Decompression;
+      return std::make_unique<Gzip_Decompression>();
    if(name == "Deflate" || name == "deflate")
-      return new Deflate_Decompression;
+      return std::make_unique<Deflate_Decompression>();
 #endif
 
 #if defined(BOTAN_HAS_BZIP2)
    if(name == "bzip2" || name == "bz2" || name == "Bzip2")
-      return new Bzip2_Decompression;
+      return std::make_unique<Bzip2_Decompression>();
 #endif
 
 #if defined(BOTAN_HAS_LZMA)
    if(name == "lzma" || name == "xz" || name == "LZMA")
-      return new LZMA_Decompression;
+      return std::make_unique<LZMA_Decompression>();
 #endif
 
    BOTAN_UNUSED(name);
    return nullptr;
    }
 
-//static
-std::unique_ptr<Decompression_Algorithm>
-Decompression_Algorithm::create(const std::string& algo)
-   {
-   std::unique_ptr<Decompression_Algorithm> decompressor(make_decompressor(algo));
-   return decompressor;
-   }
-
 //static
 std::unique_ptr<Decompression_Algorithm>
 Decompression_Algorithm::create_or_throw(const std::string& algo)
diff --git a/src/lib/compression/compression.h b/src/lib/compression/compression.h
index 217fae623..4a8ec1675 100644
--- a/src/lib/compression/compression.h
+++ b/src/lib/compression/compression.h
@@ -131,8 +131,15 @@ class BOTAN_PUBLIC_API(2,0) Decompression_Algorithm
       virtual ~Decompression_Algorithm() = default;
    };
 
-BOTAN_PUBLIC_API(2,0) Compression_Algorithm* make_compressor(const std::string& type);
-BOTAN_PUBLIC_API(2,0) Decompression_Algorithm* make_decompressor(const std::string& type);
+inline Compression_Algorithm* make_compressor(const std::string& type)
+   {
+   return Compression_Algorithm::create(type).release();
+   }
+
+inline Decompression_Algorithm* make_decompressor(const std::string& type)
+   {
+   return Decompression_Algorithm::create(type).release();
+   }
 
 /**
 * An error that occurred during compression (or decompression)
@@ -204,7 +211,7 @@ class Stream_Compression : public Compression_Algorithm
 
       void process(secure_vector<uint8_t>& buf, size_t offset, uint32_t flags);
 
-      virtual Compression_Stream* make_stream(size_t level) const = 0;
+      virtual std::unique_ptr<Compression_Stream> make_stream(size_t level) const = 0;
 
       secure_vector<uint8_t> m_buffer;
       std::unique_ptr<Compression_Stream> m_stream;
@@ -227,7 +234,7 @@ class Stream_Decompression : public Decompression_Algorithm
 
       void process(secure_vector<uint8_t>& buf, size_t offset, uint32_t flags);
 
-      virtual Compression_Stream* make_stream() const = 0;
+      virtual std::unique_ptr<Compression_Stream> make_stream() const = 0;
 
       secure_vector<uint8_t> m_buffer;
       std::unique_ptr<Compression_Stream> m_stream;
diff --git a/src/lib/compression/lzma/lzma.cpp b/src/lib/compression/lzma/lzma.cpp
index 680884c0e..7cdeb9ff2 100644
--- a/src/lib/compression/lzma/lzma.cpp
+++ b/src/lib/compression/lzma/lzma.cpp
@@ -82,14 +82,14 @@ class LZMA_Decompression_Stream final : public LZMA_Stream
 
 }
 
-Compression_Stream* LZMA_Compression::make_stream(size_t level) const
+std::unique_ptr<Compression_Stream> LZMA_Compression::make_stream(size_t level) const
    {
-   return new LZMA_Compression_Stream(level);
+   return std::make_unique<LZMA_Compression_Stream>(level);
    }
 
-Compression_Stream* LZMA_Decompression::make_stream() const
+std::unique_ptr<Compression_Stream> LZMA_Decompression::make_stream() const
    {
-   return new LZMA_Decompression_Stream;
+   return std::make_unique<LZMA_Decompression_Stream>();
    }
 
 }
diff --git a/src/lib/compression/lzma/lzma.h b/src/lib/compression/lzma/lzma.h
index 02a1f8c8d..f95809bc0 100644
--- a/src/lib/compression/lzma/lzma.h
+++ b/src/lib/compression/lzma/lzma.h
@@ -23,7 +23,7 @@ class BOTAN_PUBLIC_API(2,0) LZMA_Compression final : public Stream_Compression
       std::string name() const override { return "LZMA_Compression"; }
 
    private:
-      Compression_Stream* make_stream(size_t level) const override;
+      std::unique_ptr<Compression_Stream> make_stream(size_t level) const override;
    };
 
 /**
@@ -34,7 +34,7 @@ class BOTAN_PUBLIC_API(2,0) LZMA_Decompression final : public Stream_Decompressi
    public:
       std::string name() const override { return "LZMA_Decompression"; }
    private:
-      Compression_Stream* make_stream() const override;
+      std::unique_ptr<Compression_Stream> make_stream() const override;
    };
 
 }
diff --git a/src/lib/compression/zlib/zlib.cpp b/src/lib/compression/zlib/zlib.cpp
index 92842a6e1..61e02a89e 100644
--- a/src/lib/compression/zlib/zlib.cpp
+++ b/src/lib/compression/zlib/zlib.cpp
@@ -140,34 +140,34 @@ class Gzip_Decompression_Stream final : public Zlib_Decompression_Stream
 
 }
 
-Compression_Stream* Zlib_Compression::make_stream(size_t level) const
+std::unique_ptr<Compression_Stream> Zlib_Compression::make_stream(size_t level) const
    {
-   return new Zlib_Compression_Stream(level, 15);
+   return std::make_unique<Zlib_Compression_Stream>(level, 15);
    }
 
-Compression_Stream* Zlib_Decompression::make_stream() const
+std::unique_ptr<Compression_Stream> Zlib_Decompression::make_stream() const
    {
-   return new Zlib_Decompression_Stream(15);
+   return std::make_unique<Zlib_Decompression_Stream>(15);
    }
 
-Compression_Stream* Deflate_Compression::make_stream(size_t level) const
+std::unique_ptr<Compression_Stream> Deflate_Compression::make_stream(size_t level) const
    {
-   return new Deflate_Compression_Stream(level, 15);
+   return std::make_unique<Deflate_Compression_Stream>(level, 15);
    }
 
-Compression_Stream* Deflate_Decompression::make_stream() const
+std::unique_ptr<Compression_Stream> Deflate_Decompression::make_stream() const
    {
-   return new Deflate_Decompression_Stream(15);
+   return std::make_unique<Deflate_Decompression_Stream>(15);
    }
 
-Compression_Stream* Gzip_Compression::make_stream(size_t level) const
+std::unique_ptr<Compression_Stream> Gzip_Compression::make_stream(size_t level) const
    {
-   return new Gzip_Compression_Stream(level, 15, m_os_code, m_hdr_time);
+   return std::make_unique<Gzip_Compression_Stream>(level, 15, m_os_code, m_hdr_time);
    }
 
-Compression_Stream* Gzip_Decompression::make_stream() const
+std::unique_ptr<Compression_Stream> Gzip_Decompression::make_stream() const
    {
-   return new Gzip_Decompression_Stream(15);
+   return std::make_unique<Gzip_Decompression_Stream>(15);
    }
 
 }
diff --git a/src/lib/compression/zlib/zlib.h b/src/lib/compression/zlib/zlib.h
index cc0060344..d0293217b 100644
--- a/src/lib/compression/zlib/zlib.h
+++ b/src/lib/compression/zlib/zlib.h
@@ -21,7 +21,7 @@ class BOTAN_PUBLIC_API(2,0) Zlib_Compression final : public Stream_Compression
    public:
       std::string name() const override { return "Zlib_Compression"; }
    private:
-      Compression_Stream* make_stream(size_t level) const override;
+      std::unique_ptr<Compression_Stream> make_stream(size_t level) const override;
    };
 
 /**
@@ -32,7 +32,7 @@ class BOTAN_PUBLIC_API(2,0) Zlib_Decompression final : public Stream_Decompressi
    public:
       std::string name() const override { return "Zlib_Decompression"; }
    private:
-      Compression_Stream* make_stream() const override;
+      std::unique_ptr<Compression_Stream> make_stream() const override;
    };
 
 /**
@@ -43,7 +43,7 @@ class BOTAN_PUBLIC_API(2,0) Deflate_Compression final : public Stream_Compressio
    public:
       std::string name() const override { return "Deflate_Compression"; }
    private:
-      Compression_Stream* make_stream(size_t level) const override;
+      std::unique_ptr<Compression_Stream> make_stream(size_t level) const override;
    };
 
 /**
@@ -54,7 +54,7 @@ class BOTAN_PUBLIC_API(2,0) Deflate_Decompression final : public Stream_Decompre
    public:
       std::string name() const override { return "Deflate_Decompression"; }
    private:
-      Compression_Stream* make_stream() const override;
+      std::unique_ptr<Compression_Stream> make_stream() const override;
    };
 
 /**
@@ -68,7 +68,7 @@ class BOTAN_PUBLIC_API(2,0) Gzip_Compression final : public Stream_Compression
 
       std::string name() const override { return "Gzip_Compression"; }
    private:
-      Compression_Stream* make_stream(size_t level) const override;
+      std::unique_ptr<Compression_Stream> make_stream(size_t level) const override;
       const uint64_t m_hdr_time;
       const uint8_t m_os_code;
    };
@@ -81,7 +81,7 @@ class BOTAN_PUBLIC_API(2,0) Gzip_Decompression final : public Stream_Decompressi
    public:
       std::string name() const override { return "Gzip_Decompression"; }
    private:
-      Compression_Stream* make_stream() const override;
+      std::unique_ptr<Compression_Stream> make_stream() const override;
    };
 
 }
diff --git a/src/lib/filters/comp_filter.cpp b/src/lib/filters/comp_filter.cpp
index c2814460a..a17d723f7 100644
--- a/src/lib/filters/comp_filter.cpp
+++ b/src/lib/filters/comp_filter.cpp
@@ -18,7 +18,7 @@ namespace Botan {
 #if defined(BOTAN_HAS_COMPRESSION)
 
 Compression_Filter::Compression_Filter(const std::string& type, size_t level, size_t bs) :
-   m_comp(make_compressor(type)),
+   m_comp(Compression_Algorithm::create(type)),
    m_buffersize(std::max<size_t>(bs, 256)),
    m_level(level)
    {
@@ -72,7 +72,7 @@ void Compression_Filter::end_msg()
    }
 
 Decompression_Filter::Decompression_Filter(const std::string& type, size_t bs) :
-   m_comp(make_decompressor(type)),
+   m_comp(Decompression_Algorithm::create(type)),
    m_buffersize(std::max<size_t>(bs, 256))
    {
    if(!m_comp)


Test patch:
diff --git a/src/tests/test_compression.cpp b/src/tests/test_compression.cpp
index 7f503d538..bc21fbeb6 100644
--- a/src/tests/test_compression.cpp
+++ b/src/tests/test_compression.cpp
@@ -68,8 +68,8 @@ class Compression_Tests final : public Test
 
                result.start_timer();
 
-               std::unique_ptr<Botan::Compression_Algorithm> c(Botan::make_compressor(algo));
-               std::unique_ptr<Botan::Decompression_Algorithm> d(Botan::make_decompressor(algo));
+               auto c = Botan::Compression_Algorithm::create(algo);
+               auto d = Botan::Decompression_Algorithm::create(algo);
 
                if(!c || !d)
                   {
diff --git a/src/tests/test_entropy.cpp b/src/tests/test_entropy.cpp
index 9aac7d24d..8bc3d313d 100644
--- a/src/tests/test_entropy.cpp
+++ b/src/tests/test_entropy.cpp
@@ -58,7 +58,7 @@ class Entropy_Source_Tests final : public Test
                   */
                   for(const std::string comp_algo : { "zlib", "lzma" })
                      {
-                     std::unique_ptr<Botan::Compression_Algorithm> comp(Botan::make_compressor(comp_algo));
+                     auto comp = Botan::Compression_Algorithm::create(comp_algo);
 
                      if(comp)
                         {