Platform::GetReader returns unique_ptr. Fix leaks.

12 files changed, 57 insertions, 52 deletions

diff --git a/coding/buffer_reader.hpp b/coding/buffer_reader.hpp
index 8c99d644d1..c35911a29a 100644
--- a/coding/buffer_reader.hpp
+++ b/coding/buffer_reader.hpp
@@ -40,9 +40,10 @@ public:
     return BufferReader(*this, pos, size);
   }
 
-  inline BufferReader * CreateSubReader(uint64_t pos, uint64_t size) const
+  inline unique_ptr<Reader> CreateSubReader(uint64_t pos, uint64_t size) const
   {
-    return new BufferReader(*this, pos, size);
+    // Can't use make_unique with private constructor.
+    return unique_ptr<Reader>(new BufferReader(*this, pos, size));
   }
 
 private:
diff --git a/coding/coding_tests/file_container_test.cpp b/coding/coding_tests/file_container_test.cpp
index e1c657d086..fa473a3802 100644
--- a/coding/coding_tests/file_container_test.cpp
+++ b/coding/coding_tests/file_container_test.cpp
@@ -33,8 +33,8 @@ UNIT_TEST(FilesContainer_Smoke)
 
     for (size_t i = 0; i < count; ++i)
     {
-      FilesContainerR::ReaderT r = reader.GetReader(strings::to_string(i));
-      ReaderSource<FilesContainerR::ReaderT> src(r);
+      FilesContainerR::TReader r = reader.GetReader(strings::to_string(i));
+      ReaderSource<FilesContainerR::TReader> src(r);
 
       for (uint32_t j = 0; j < i; ++j)
       {
@@ -59,8 +59,8 @@ UNIT_TEST(FilesContainer_Smoke)
     {
       FilesContainerR reader(fName);
 
-      FilesContainerR::ReaderT r = reader.GetReader(strings::to_string(arrAppend[i]));
-      ReaderSource<FilesContainerR::ReaderT> src(r);
+      FilesContainerR::TReader r = reader.GetReader(strings::to_string(arrAppend[i]));
+      ReaderSource<FilesContainerR::TReader> src(r);
 
       uint32_t const test = ReadVarUint<uint32_t>(src);
       TEST_EQUAL(arrAppend[i], test, ());
@@ -73,7 +73,7 @@ namespace
 {
   void CheckInvariant(FilesContainerR & reader, string const & tag, int64_t test)
   {
-    FilesContainerR::ReaderT r = reader.GetReader(tag);
+    FilesContainerR::TReader r = reader.GetReader(tag);
     TEST_EQUAL(test, ReadPrimitiveFromPos<int64_t>(r, 0), ());
   }
 }
@@ -107,7 +107,7 @@ UNIT_TEST(FilesContainer_Shared)
     // shared container read and fill
 
     FilesContainerR reader(fName);
-    FilesContainerR::ReaderT r1 = reader.GetReader("5");
+    FilesContainerR::TReader r1 = reader.GetReader("5");
     uint64_t const offset = sizeof(uint32_t);
     r1 = r1.SubReader(offset, r1.Size() - offset);
 
@@ -116,7 +116,7 @@ UNIT_TEST(FilesContainer_Shared)
     FilesContainerW writer(fName, FileWriter::OP_WRITE_EXISTING);
     FileWriter w = writer.GetWriter("3");
 
-    ReaderSource<FilesContainerR::ReaderT> src(r1);
+    ReaderSource<FilesContainerR::TReader> src(r1);
     for (uint32_t i = 0; i < count; ++i)
     {
       uint32_t test = ReadVarUint<uint32_t>(src);
@@ -152,7 +152,7 @@ namespace
 
     for (size_t i = 0; i < count; ++i)
     {
-      FilesContainerR::ReaderT r = reader.GetReader(key[i]);
+      FilesContainerR::TReader r = reader.GetReader(key[i]);
 
       size_t const szBuffer = 100;
       size_t const szS = strlen(value[i]);
diff --git a/coding/coding_tests/reader_test.cpp b/coding/coding_tests/reader_test.cpp
index 94dba654d9..64f910b923 100644
--- a/coding/coding_tests/reader_test.cpp
+++ b/coding/coding_tests/reader_test.cpp
@@ -113,7 +113,7 @@ UNIT_TEST(ReaderStreamBuf)
   }
 
   {
-    ReaderStreamBuf buffer(new FileReader(name));
+    ReaderStreamBuf buffer(make_unique<FileReader>(name));
     istream s(&buffer);
 
     std::string str;
diff --git a/coding/file_container.cpp b/coding/file_container.cpp
index fab8710331..4a1045687a 100644
--- a/coding/file_container.cpp
+++ b/coding/file_container.cpp
@@ -64,18 +64,18 @@ void FilesContainerBase::ReadInfo(ReaderT & reader)
 FilesContainerR::FilesContainerR(string const & filePath,
                                  uint32_t logPageSize,
                                  uint32_t logPageCount)
-  : m_source(new FileReader(filePath, logPageSize, logPageCount))
+  : m_source(make_unique<FileReader>(filePath, logPageSize, logPageCount))
 {
   ReadInfo(m_source);
 }
 
-FilesContainerR::FilesContainerR(ReaderT const & file)
+FilesContainerR::FilesContainerR(TReader const & file)
   : m_source(file)
 {
   ReadInfo(m_source);
 }
 
-FilesContainerR::ReaderT FilesContainerR::GetReader(Tag const & tag) const
+FilesContainerR::TReader FilesContainerR::GetReader(Tag const & tag) const
 {
   Info const * p = GetInfo(tag);
   if (!p)
@@ -413,7 +413,7 @@ FileWriter FilesContainerW::GetWriter(Tag const & tag)
 
 void FilesContainerW::Write(string const & fPath, Tag const & tag)
 {
-  Write(new FileReader(fPath), tag);
+  Write(ModelReaderPtr(make_unique<FileReader>(fPath)), tag);
 }
 
 void FilesContainerW::Write(ModelReaderPtr reader, Tag const & tag)
diff --git a/coding/file_container.hpp b/coding/file_container.hpp
index ea81e7d970..50458450f5 100644
--- a/coding/file_container.hpp
+++ b/coding/file_container.hpp
@@ -105,14 +105,14 @@ public:
 class FilesContainerR : public FilesContainerBase
 {
 public:
-  typedef ModelReaderPtr ReaderT;
+  using TReader = ModelReaderPtr;
 
   explicit FilesContainerR(string const & filePath,
                            uint32_t logPageSize = 10,
                            uint32_t logPageCount = 10);
-  explicit FilesContainerR(ReaderT const & file);
+  explicit FilesContainerR(TReader const & file);
 
-  ReaderT GetReader(Tag const & tag) const;
+  TReader GetReader(Tag const & tag) const;
 
   template <typename F> void ForEachTag(F f) const
   {
@@ -126,12 +126,11 @@ public:
   pair<uint64_t, uint64_t> GetAbsoluteOffsetAndSize(Tag const & tag) const;
 
 private:
-  ReaderT m_source;
+  TReader m_source;
 };
 
 namespace detail
 {
-
 class MappedFile
 {
   DISALLOW_COPY(MappedFile);
diff --git a/coding/file_reader.cpp b/coding/file_reader.cpp
index d0300bccd2..1ca0aff021 100644
--- a/coding/file_reader.cpp
+++ b/coding/file_reader.cpp
@@ -96,10 +96,11 @@ FileReader FileReader::SubReader(uint64_t pos, uint64_t size) const
   return FileReader(*this, m_Offset + pos, size);
 }
 
-FileReader * FileReader::CreateSubReader(uint64_t pos, uint64_t size) const
+unique_ptr<Reader> FileReader::CreateSubReader(uint64_t pos, uint64_t size) const
 {
   ASSERT ( AssertPosAndSize(pos, size), () );
-  return new FileReader(*this, m_Offset + pos, size);
+  // Can't use make_unique with private constructor.
+  return unique_ptr<Reader>(new FileReader(*this, m_Offset + pos, size));
 }
 
 bool FileReader::AssertPosAndSize(uint64_t pos, uint64_t size) const
diff --git a/coding/file_reader.hpp b/coding/file_reader.hpp
index 6008452f33..882b07b019 100644
--- a/coding/file_reader.hpp
+++ b/coding/file_reader.hpp
@@ -17,10 +17,10 @@ public:
 
   class FileReaderData;
 
-  uint64_t Size() const;
-  void Read(uint64_t pos, void * p, size_t size) const;
+  uint64_t Size() const override;
+  void Read(uint64_t pos, void * p, size_t size) const override;
   FileReader SubReader(uint64_t pos, uint64_t size) const;
-  FileReader * CreateSubReader(uint64_t pos, uint64_t size) const;
+  unique_ptr<Reader> CreateSubReader(uint64_t pos, uint64_t size) const override;
 
   inline uint64_t GetOffset() const { return m_Offset; }
 
diff --git a/coding/mmap_reader.cpp b/coding/mmap_reader.cpp
index 4d0f677dbc..53478fba0c 100644
--- a/coding/mmap_reader.cpp
+++ b/coding/mmap_reader.cpp
@@ -79,10 +79,11 @@ void MmapReader::Read(uint64_t pos, void * p, size_t size) const
   memcpy(p, m_data->m_memory + m_offset + pos, size);
 }
 
-MmapReader * MmapReader::CreateSubReader(uint64_t pos, uint64_t size) const
+unique_ptr<Reader> MmapReader::CreateSubReader(uint64_t pos, uint64_t size) const
 {
   ASSERT_LESS_OR_EQUAL(pos + size, Size(), (pos, size));
-  return new MmapReader(*this, m_offset + pos, size);
+  // Can't use make_unique with private constructor.
+  return unique_ptr<Reader>(new MmapReader(*this, m_offset + pos, size));
 }
 
 uint8_t * MmapReader::Data() const
diff --git a/coding/mmap_reader.hpp b/coding/mmap_reader.hpp
index 7e1a9d059b..8b41f66ec1 100644
--- a/coding/mmap_reader.hpp
+++ b/coding/mmap_reader.hpp
@@ -19,9 +19,9 @@ class MmapReader : public ModelReader
 public:
   explicit MmapReader(string const & fileName);
 
-  virtual uint64_t Size() const;
-  virtual void Read(uint64_t pos, void * p, size_t size) const;
-  virtual MmapReader * CreateSubReader(uint64_t pos, uint64_t size) const;
+  uint64_t Size() const override;
+  void Read(uint64_t pos, void * p, size_t size) const override;
+  unique_ptr<Reader> CreateSubReader(uint64_t pos, uint64_t size) const override;
 
   /// Direct file/memory access
   uint8_t * Data() const;
diff --git a/coding/reader.hpp b/coding/reader.hpp
index 09a1b73ec8..968395447e 100644
--- a/coding/reader.hpp
+++ b/coding/reader.hpp
@@ -4,10 +4,11 @@
 #include "base/assert.hpp"
 #include "base/exception.hpp"
 
+#include "std/cstring.hpp"
 #include "std/shared_array.hpp"
 #include "std/shared_ptr.hpp"
 #include "std/string.hpp"
-#include "std/cstring.hpp"
+#include "std/unique_ptr.hpp"
 #include "std/vector.hpp"
 
 // Base class for random-access Reader. Not thread-safe.
@@ -19,10 +20,12 @@ public:
   DECLARE_EXCEPTION(SizeException, Exception);
   DECLARE_EXCEPTION(ReadException, Exception);
 
+  using TReaderPtr = unique_ptr<Reader>;
+
   virtual ~Reader() {}
   virtual uint64_t Size() const = 0;
   virtual void Read(uint64_t pos, void * p, size_t size) const = 0;
-  virtual Reader * CreateSubReader(uint64_t pos, uint64_t size) const = 0;
+  virtual TReaderPtr CreateSubReader(uint64_t pos, uint64_t size) const = 0;
 
   void ReadAsString(string & s) const;
 
@@ -41,12 +44,12 @@ public:
   {
   }
 
-  inline uint64_t Size() const
+  inline uint64_t Size() const override
   {
     return m_size;
   }
 
-  inline void Read(uint64_t pos, void * p, size_t size) const
+  inline void Read(uint64_t pos, void * p, size_t size) const override
   {
     ASSERT ( AssertPosAndSize(pos, size), () );
     memcpy(p, m_pData + pos, size);
@@ -58,10 +61,10 @@ public:
     return MemReader(m_pData + pos, static_cast<size_t>(size));
   }
 
-  inline MemReader * CreateSubReader(uint64_t pos, uint64_t size) const
+  inline unique_ptr<Reader> CreateSubReader(uint64_t pos, uint64_t size) const override
   {
     ASSERT ( AssertPosAndSize(pos, size), () );
-    return new MemReader(m_pData + pos, static_cast<size_t>(size));
+    return make_unique<MemReader>(m_pData + pos, static_cast<size_t>(size));
   }
 
 private:
@@ -92,6 +95,7 @@ public:
     return SharedMemReader(m_data, static_cast<size_t>(pos), static_cast<size_t>(size));
   }
 
+  // TODO(mgsergio): return unique_ptr
   inline SharedMemReader * CreateSubReader(uint64_t pos, uint64_t size) const
   {
     ASSERT ( AssertPosAndSize(pos, size), () );
@@ -117,7 +121,8 @@ protected:
   shared_ptr<TReader> m_p;
 
 public:
-  ReaderPtr(TReader * p = 0) : m_p(p) {}
+  template <typename TReaderDerived>
+  ReaderPtr(unique_ptr<TReaderDerived> p) : m_p(move(p)) {}
 
   uint64_t Size() const
   {
@@ -145,7 +150,7 @@ class ModelReader : public Reader
 public:
   ModelReader(string const & name) : m_name(name) {}
 
-  virtual ModelReader * CreateSubReader(uint64_t pos, uint64_t size) const = 0;
+  virtual unique_ptr<Reader> CreateSubReader(uint64_t pos, uint64_t size) const override = 0;
 
   inline string const & GetName() const { return m_name; }
 };
@@ -153,26 +158,26 @@ public:
 // Reader pointer class for data files.
 class ModelReaderPtr : public ReaderPtr<ModelReader>
 {
-  typedef ReaderPtr<ModelReader> base_type;
+  using TBase = ReaderPtr<ModelReader>;
 
 public:
-  ModelReaderPtr(ModelReader * p) : base_type(p) {}
+  template <typename TReaderDerived>
+  ModelReaderPtr(unique_ptr<TReaderDerived> p) : TBase(move(p)) {}
 
   inline ModelReaderPtr SubReader(uint64_t pos, uint64_t size) const
   {
-    return m_p->CreateSubReader(pos, size);
+    return unique_ptr<ModelReader>(static_cast<ModelReader *>(m_p->CreateSubReader(pos, size).release()));
   }
 
   inline string const & GetName() const { return m_p->GetName(); }
 };
 
-
 // Source that reads from a reader.
 template <typename TReader>
 class ReaderSource
 {
 public:
-  typedef TReader ReaderType;
+  using ReaderType = TReader;
 
   ReaderSource(TReader const & reader) : m_reader(reader), m_pos(0) {}
 
diff --git a/coding/reader_streambuf.cpp b/coding/reader_streambuf.cpp
index 3a023f323e..859af2eb70 100644
--- a/coding/reader_streambuf.cpp
+++ b/coding/reader_streambuf.cpp
@@ -5,15 +5,13 @@
 #include "std/algorithm.hpp"
 
 
-ReaderStreamBuf::ReaderStreamBuf(Reader * p)
-: m_p(p), m_pos(0), m_size(p->Size())
+ReaderStreamBuf::ReaderStreamBuf(unique_ptr<Reader> && p)
+  : m_p(move(p)), m_pos(0), m_size(m_p->Size())
 {
 }
 
-ReaderStreamBuf::~ReaderStreamBuf()
-{
-  delete m_p;
-}
+// Define destructor in .cpp due to using unique_ptr with incomplete type.
+ReaderStreamBuf::~ReaderStreamBuf() = default;
 
 std::streamsize ReaderStreamBuf::xsgetn(char_type * s, std::streamsize n)
 {
diff --git a/coding/reader_streambuf.hpp b/coding/reader_streambuf.hpp
index 6b2ad4db75..a253e9531f 100644
--- a/coding/reader_streambuf.hpp
+++ b/coding/reader_streambuf.hpp
@@ -2,6 +2,7 @@
 
 #include "std/cstdint.hpp"
 #include "std/iostream.hpp"
+#include "std/unique_ptr.hpp"
 
 class Reader;
 class Writer;
@@ -17,12 +18,11 @@ public:
 
 class ReaderStreamBuf : public BaseStreamBuf
 {
-  Reader * m_p;
+  unique_ptr<Reader> m_p;
   uint64_t m_pos, m_size;
 
 public:
-  /// Takes the ownership of p. Reader should be allocated in dynamic memory.
-  ReaderStreamBuf(Reader * p);
+  ReaderStreamBuf(unique_ptr<Reader> && p);
   virtual ~ReaderStreamBuf();
 
 private: