zlibstream classes: add constructors with sink semantic based on std::unique_ptr

Add an alternate constructor to each of the following classes:
ZlibAbstractIStreambuf, ZlibCompressorIStreambuf,
ZlibDecompressorIStreambuf, ZlibCompressorIStream and
ZlibDecompressorIStream. These new constructors are passed the source
std::istream wrapped inside an std::unique_ptr instead of by reference,
and store the unique_ptr object as an instance member. This ensures that
the source std::istream object is available as long as the
ZlibDecompressorIStreambuf, etc. instance is alive (which is necessary
for its getInputData() method) without any additional work for callers,
and that it is automatically destroyed afterwards.

This is particularly useful when writing functions that create and
return an object 'zobj' whose type is a subclass of
ZlibAbstractIStreambuf, when the source std::istream is only of interest
for its role of feeding data to 'zobj'. For instance:

std::unique_ptr<simgear::ZlibCompressorIStream>
myZlibCompressorIStreamProducer(std::string str)
{
  std::unique_ptr<std::istringstream> iss(new std::istringstream(str));

  return std::unique_ptr<simgear::ZlibCompressorIStream>(
    new simgear::ZlibCompressorIStream(std::move(iss))); // new ctor here
}

Callers of such a function get access to a new ZlibCompressorIStream
instance fed by an std::istringstream object ('iss'), but they don't
even have to know this detail, nor to take any measure to ensure that
'iss' lives at least as long as the ZlibCompressorIStream object. The
std::unique_ptr<std::istream> pointing to 'iss' and stored as a member
of the ZlibCompressorIStream object by its constructor automatically
takes care of this lifetime problem.

simgear/io/iostreams/zlibstream.cxx

@@ -23,6 +23,8 @@
 #include <string>
 #include <ios>                  // std::streamsize
 #include <istream>
+#include <memory>               // std::unique_ptr
+#include <utility>              // std::move()
 #include <algorithm>
 #include <stdexcept>
 #include <unordered_map>
@@ -141,13 +143,13 @@ ZlibAbstractIStreambuf::ZlibAbstractIStreambuf(std::istream& iStream,
                                                std::size_t outBufSize,
                                                std::size_t putbackSize)
   : _iStream(iStream),
+    _iStream_p(nullptr),
     _path(path),
     _inBuf(inBuf),
     _inBufSize(inBufSize),
     _outBuf(outBuf),
     _outBufSize(outBufSize),
     _putbackSize(putbackSize)
-
 {
   assert(_inBufSize > 0);
   assert(_putbackSize >= 0);    // guaranteed unless the type is changed...
@@ -180,6 +182,24 @@ ZlibAbstractIStreambuf::ZlibAbstractIStreambuf(std::istream& iStream,
   setg(_outBuf, _outBuf, _outBuf);
 }
 
+ZlibAbstractIStreambuf::ZlibAbstractIStreambuf(
+  std::unique_ptr<std::istream> iStream_p,
+  const SGPath& path,
+  char* inBuf,
+  std::size_t inBufSize,
+  char *outBuf,
+  std::size_t outBufSize,
+  std::size_t putbackSize)
+  : ZlibAbstractIStreambuf(*iStream_p, path, inBuf, inBufSize,
+                           outBuf, outBufSize, putbackSize)
+{
+  // Take ownership of the object. This is a way to ensure that the _iStream
+  // reference stays valid as long as our instance is alive, and that the
+  // corresponding std::istream object is automatically destroyed as soon as
+  // our instance is itself destroyed.
+  _iStream_p = std::move(iStream_p);
+}
+
 ZlibAbstractIStreambuf::~ZlibAbstractIStreambuf()
 {
   if (_inBufMustBeFreed) {
@@ -554,6 +574,24 @@ ZlibCompressorIStreambuf::ZlibCompressorIStreambuf(
   zStreamInit(compressionLevel, format, memStrategy);
 }
 
+ZlibCompressorIStreambuf::ZlibCompressorIStreambuf(
+  std::unique_ptr<std::istream> iStream_p,
+  const SGPath& path,
+  int compressionLevel,
+  ZLibCompressionFormat format,
+  ZLibMemoryStrategy memStrategy,
+  char* inBuf,
+  std::size_t inBufSize,
+  char *outBuf,
+  std::size_t outBufSize,
+  std::size_t putbackSize)
+  : ZlibCompressorIStreambuf(*iStream_p, path, compressionLevel, format,
+                             memStrategy, inBuf, inBufSize, outBuf, outBufSize,
+                             putbackSize)
+{
+  _iStream_p = std::move(iStream_p); // take ownership of the object
+}
+
 ZlibCompressorIStreambuf::~ZlibCompressorIStreambuf()
 {
   int retCode = deflateEnd(&_zstream); // deallocate the z_stream struct
@@ -642,6 +680,21 @@ ZlibDecompressorIStreambuf::ZlibDecompressorIStreambuf(
   zStreamInit(format);
 }
 
+ZlibDecompressorIStreambuf::ZlibDecompressorIStreambuf(
+  std::unique_ptr<std::istream> iStream_p,
+  const SGPath& path,
+  ZLibCompressionFormat format,
+  char* inBuf,
+  std::size_t inBufSize,
+  char *outBuf,
+  std::size_t outBufSize,
+  std::size_t putbackSize)
+  : ZlibDecompressorIStreambuf(*iStream_p, path, format, inBuf, inBufSize,
+                               outBuf, outBufSize, putbackSize)
+{
+  _iStream_p = std::move(iStream_p); // take ownership of the object
+}
+
 ZlibDecompressorIStreambuf::~ZlibDecompressorIStreambuf()
 {
   int retCode = inflateEnd(&_zstream); // deallocate the z_stream struct
@@ -719,6 +772,25 @@ ZlibCompressorIStream::ZlibCompressorIStream(std::istream& iStream,
   rdbuf(&_streamBuf);
 }
 
+ZlibCompressorIStream::ZlibCompressorIStream(
+  std::unique_ptr<std::istream> iStream_p,
+  const SGPath& path,
+  int compressionLevel,
+  ZLibCompressionFormat format,
+  ZLibMemoryStrategy memStrategy,
+  char* inBuf,
+  std::size_t inBufSize,
+  char *outBuf,
+  std::size_t outBufSize,
+  std::size_t putbackSize)
+  : std::istream(nullptr),
+    _streamBuf(std::move(iStream_p), path, compressionLevel, format,
+               memStrategy, inBuf, inBufSize, outBuf, outBufSize, putbackSize)
+{
+  // Associate _streamBuf to 'this' and clear the error state flags
+  rdbuf(&_streamBuf);
+}
+
 ZlibCompressorIStream::~ZlibCompressorIStream()
 { }
 
@@ -742,6 +814,23 @@ ZlibDecompressorIStream::ZlibDecompressorIStream(std::istream& iStream,
   rdbuf(&_streamBuf);
 }
 
+ZlibDecompressorIStream::ZlibDecompressorIStream(
+  std::unique_ptr<std::istream> iStream_p,
+  const SGPath& path,
+  ZLibCompressionFormat format,
+  char* inBuf,
+  std::size_t inBufSize,
+  char *outBuf,
+  std::size_t outBufSize,
+  std::size_t putbackSize)
+  : std::istream(nullptr),
+    _streamBuf(std::move(iStream_p), path, format, inBuf, inBufSize,
+               outBuf, outBufSize, putbackSize)
+{
+  // Associate _streamBuf to 'this' and clear the error state flags
+  rdbuf(&_streamBuf);
+}
+
 ZlibDecompressorIStream::~ZlibDecompressorIStream()
 { }
 

simgear/io/iostreams/zlibstream.hxx

@@ -25,6 +25,7 @@
 
 #include <iosfwd>
 #include <ios>                  // std::streamsize
+#include <memory>               // std::unique_ptr
 #include <zlib.h>               // struct z_stream
 
 #include <simgear/misc/sg_path.hxx>
@@ -144,6 +145,19 @@ public:
                                   char* outBuf = nullptr,
                                   std::size_t outBufSize = 262144,
                                   std::size_t putbackSize = 0);
+
+  // Alternate constructor with sink semantics for the “source” std::istream.
+  // When used, the class takes ownership of the std::istream instance pointed
+  // to by the first constructor argument, and keeps it alive as long as the
+  // object this constructor is for is itself alive.
+  explicit ZlibAbstractIStreambuf(std::unique_ptr<std::istream> iStream_p,
+                                  const SGPath& path = SGPath(),
+                                  char* inBuf = nullptr,
+                                  std::size_t inBufSize = 262144,
+                                  char* outBuf = nullptr,
+                                  std::size_t outBufSize = 262144,
+                                  std::size_t putbackSize = 0);
+
   ZlibAbstractIStreambuf(const ZlibAbstractIStreambuf&) = delete;
   ZlibAbstractIStreambuf& operator=(const ZlibAbstractIStreambuf&) = delete;
   ~ZlibAbstractIStreambuf();
@@ -166,6 +180,11 @@ protected:
 
   // The input stream, from which data is read before being processed by zlib
   std::istream& _iStream;
+  // Pointer to the same, used when calling the constructor that takes an
+  // std::unique_ptr<std::istream> as its first argument; empty
+  // std::unique_ptr object otherwise.
+  std::unique_ptr<std::istream> _iStream_p;
+
   // Corresponding path, if any (default-constructed SGPath instance otherwise)
   const SGPath _path;
   // Structure used to communicate with zlib
@@ -285,6 +304,20 @@ public:
     char* outBuf = nullptr,
     std::size_t outBufSize = 262144,
     std::size_t putbackSize = 0);
+
+  // Alternate constructor with sink semantics for the “source” std::istream.
+  explicit ZlibCompressorIStreambuf(
+    std::unique_ptr<std::istream> _iStream_p,
+    const SGPath& path = SGPath(),
+    int compressionLevel = Z_DEFAULT_COMPRESSION,
+    ZLibCompressionFormat format = ZLIB_COMPRESSION_FORMAT_ZLIB,
+    ZLibMemoryStrategy memStrategy = ZLIB_FAVOR_SPEED_OVER_MEMORY,
+    char* inBuf = nullptr,
+    std::size_t inBufSize = 262144,
+    char* outBuf = nullptr,
+    std::size_t outBufSize = 262144,
+    std::size_t putbackSize = 0);
+
   ZlibCompressorIStreambuf(const ZlibCompressorIStreambuf&) = delete;
   ZlibCompressorIStreambuf& operator=(const ZlibCompressorIStreambuf&) = delete;
   ~ZlibCompressorIStreambuf();
@@ -322,6 +355,18 @@ public:
     char* outBuf = nullptr,
     std::size_t outBufSize = 262144,
     std::size_t putbackSize = 0); // default optimized for speed
+
+  // Alternate constructor with sink semantics for the “source” std::istream.
+  explicit ZlibDecompressorIStreambuf(
+    std::unique_ptr<std::istream> _iStream_p,
+    const SGPath& path = SGPath(),
+    ZLibCompressionFormat format = ZLIB_COMPRESSION_FORMAT_ZLIB,
+    char* inBuf = nullptr,
+    std::size_t inBufSize = 262144,
+    char* outBuf = nullptr,
+    std::size_t outBufSize = 262144,
+    std::size_t putbackSize = 0); // default optimized for speed
+
   ZlibDecompressorIStreambuf(const ZlibDecompressorIStreambuf&) = delete;
   ZlibDecompressorIStreambuf& operator=(const ZlibDecompressorIStreambuf&)
                                                                       = delete;
@@ -359,6 +404,20 @@ public:
     char* outBuf = nullptr,
     std::size_t outBufSize = 262144,
     std::size_t putbackSize = 0); // default optimized for speed
+
+  // Alternate constructor with sink semantics for the “source” std::istream.
+  explicit ZlibCompressorIStream(
+    std::unique_ptr<std::istream> _iStream_p,
+    const SGPath& path = SGPath(),
+    int compressionLevel = Z_DEFAULT_COMPRESSION,
+    ZLibCompressionFormat format = ZLIB_COMPRESSION_FORMAT_ZLIB,
+    ZLibMemoryStrategy memStrategy = ZLIB_FAVOR_SPEED_OVER_MEMORY,
+    char* inBuf = nullptr,
+    std::size_t inBufSize = 262144,
+    char* outBuf = nullptr,
+    std::size_t outBufSize = 262144,
+    std::size_t putbackSize = 0); // default optimized for speed
+
   ZlibCompressorIStream(const ZlibCompressorIStream&) = delete;
   ZlibCompressorIStream& operator=(const ZlibCompressorIStream&) = delete;
   ~ZlibCompressorIStream();
@@ -391,6 +450,18 @@ public:
     char* outBuf = nullptr,
     std::size_t outBufSize = 262144,
     std::size_t putbackSize = 0); // default optimized for speed
+
+  // Alternate constructor with sink semantics for the “source” std::istream.
+  explicit ZlibDecompressorIStream(
+    std::unique_ptr<std::istream> _iStream_p,
+    const SGPath& path = SGPath(),
+    ZLibCompressionFormat format = ZLIB_COMPRESSION_FORMAT_ZLIB,
+    char* inBuf = nullptr,
+    std::size_t inBufSize = 262144,
+    char* outBuf = nullptr,
+    std::size_t outBufSize = 262144,
+    std::size_t putbackSize = 0); // default optimized for speed
+
   ZlibDecompressorIStream(const ZlibDecompressorIStream&) = delete;
   ZlibDecompressorIStream& operator=(const ZlibDecompressorIStream&) = delete;
   ~ZlibDecompressorIStream();

simgear/io/iostreams/zlibstream_test.cxx

@@ -24,6 +24,7 @@
 #include <array>
 #include <random>
 #include <memory>               // std::unique_ptr
+#include <utility>              // std::move()
 #include <limits>               // std::numeric_limits
 #include <type_traits>          // std::make_unsigned()
 #include <functional>           // std::bind()
@@ -63,8 +64,8 @@ static std::size_t streamsizeToSize_t(std::streamsize n)
 // you don't need the putback feature in non-test code, best performance is
 // achieved with putback size = 0.
 //
-// I suggest you read roundTripWithIStreams() below to see how to use the
-// classes most efficiently (especially the comments!).
+// I suggest reading test_IStreamConstructorWithSinkSemantics() below to see
+// how to use the classes efficiently.
 
 static std::default_random_engine randomNumbersGenerator;
 
@@ -493,7 +494,6 @@ void test_ZlibDecompressorIStream_readPutbackEtc()
 // Utility function: parametrized round-trip test with a compressor +
 // decompressor pipeline.
 //
-//
 // Note: this is nice conceptually, allows to keep memory use constant even in
 //       case an arbitrary amount of data is passed through, and exercises the
 //       stream buffer classes well, however this technique is more than twice
@@ -626,6 +626,82 @@ void test_RoundTripMultiWithIStreams()
   }
 }
 
+// Utility function showing how to return a (unique_ptr to a)
+// ZlibCompressorIStream instance, that keeps a reference to its data source
+// as long as the ZlibCompressorIStream instance is alive. Thus, calling code
+// doesn't have to worry about the lifetime of said data source (here, an
+// std::istringstream instance).
+std::unique_ptr<simgear::ZlibCompressorIStream>
+IStreamConstructorWithSinkSemantics_compressorFactory(string str)
+{
+  std::unique_ptr<std::istringstream> iss(new std::istringstream(str));
+
+  // The returned compressor object retains a “reference” (of unique_ptr type)
+  // to the std::istringstream object pointed to by 'iss' as long as it is
+  // alive. When the returned compressor object (wrapped in a unique_ptr) is
+  // destroyed, this std::istringstream object will be automatically
+  // destroyed too.
+  //
+  // Note: it's an implementation detail, but this test also indirectly
+  //       exercises the ZlibCompressorIStreambuf constructor taking an
+  //       argument of type std::unique_ptr<std::istream>.
+  return std::unique_ptr<simgear::ZlibCompressorIStream>(
+    new simgear::ZlibCompressorIStream(std::move(iss)));
+}
+
+void test_IStreamConstructorWithSinkSemantics()
+{
+  cerr << "Testing the unique_ptr-based ZlibCompressorIStream constructor\n";
+
+  string someString = randomString(4096, 8192); // arbitrary values
+  // This shows how to get a new compressor or decompressor object from a
+  // factory function. Of course, we could create the object directly on the
+  // stack without using a separate function!
+  std::unique_ptr<simgear::ZlibCompressorIStream> compressor =
+    IStreamConstructorWithSinkSemantics_compressorFactory(someString);
+  compressor->exceptions(std::ios_base::badbit); // throw if badbit is set
+
+  // Use the compressor as input to the decompressor (pipeline). The
+  // decompressor uses read() with chunks that are as large as possible given
+  // the available space in its input buffer. These read() calls are served by
+  // ZlibCompressorIStreambuf::xsgetn(), which is efficient. We won't need the
+  // compressor afterwards, so let's just std::move() its unique_ptr.
+  simgear::ZlibDecompressorIStream decompressor(std::move(compressor));
+  decompressor.exceptions(std::ios_base::badbit);
+
+  std::ostringstream roundTripResult;
+  // Of course, you may want to adjust bufSize depending on the application.
+  static constexpr std::size_t bufSize = 1024;
+  std::unique_ptr<char[]> buf(new char[bufSize]);
+
+  // Relatively efficient way of reading from the decompressor (modulo
+  // possible adjustments to 'bufSize', of course). The decompressed data is
+  // first written to 'buf', then copied to 'roundTripResult'. There is no
+  // other useless copy via, for instance, an intermediate std::string object,
+  // as would be the case if we used std::string(buf.get(), bufSize).
+  //
+  // Of course, ideally 'roundTripResult' would directly pull from
+  // 'decompressor' without going through 'buf', but I don't think this is
+  // possible with std::stringstream and friends. Such an optimized data flow
+  // is however straightforward to implement if you replace 'roundTripResult'
+  // with a custom data sink that calls decompressor.read().
+  do {
+    decompressor.read(buf.get(), bufSize);
+    if (decompressor.gcount() > 0) { // at least one char could be read
+      roundTripResult.write(buf.get(), decompressor.gcount());
+    }
+  } while (decompressor && roundTripResult);
+
+  // 1) If set, badbit would have caused an exception to be raised (see above).
+  // 2) failbit doesn't necessarily indicate an error here: it is set as soon
+  //    as the read() call can't provide the requested number of characters.
+  SG_VERIFY(decompressor.eof() && !decompressor.bad());
+  // Because of std::ostringstream::write(), 'roundTripResult' might have its
+  // failbit or badbit set, either of which would indicate a real problem.
+  SG_VERIFY(roundTripResult);
+  SG_CHECK_EQUAL(roundTripResult.str(), someString);
+}
+
 int main(int argc, char** argv)
 {
   test_pipeCompOrDecompIStreambufIntoOStream();
@@ -634,6 +710,7 @@ int main(int argc, char** argv)
   test_RoundTripMultiWithIStreams();
   test_formattedInputFromDecompressor();
   test_ZlibDecompressorIStream_readPutbackEtc();
+  test_IStreamConstructorWithSinkSemantics();
 
   return EXIT_SUCCESS;
 }