From c20b09030db4c9f0fa089d04a7d819b32e359662 Mon Sep 17 00:00:00 2001
From: Sebastian Messmer <mail@smessmer.de>
Date: Fri, 5 Dec 2014 08:08:45 +0100
Subject: [PATCH] Move load from file functionality to Data class

---
 src/blobstore/implementations/ondisk/Data.cpp | 44 ++++++++++++++++++-
 src/blobstore/implementations/ondisk/Data.h   |  8 ++++
 .../implementations/ondisk/OnDiskBlob.cpp     | 38 +++++-----------
 .../implementations/ondisk/OnDiskBlob.h       |  8 ++--
 4 files changed, 65 insertions(+), 33 deletions(-)

diff --git a/src/blobstore/implementations/ondisk/Data.cpp b/src/blobstore/implementations/ondisk/Data.cpp
index 1629a90f..fc89086f 100644
--- a/src/blobstore/implementations/ondisk/Data.cpp
+++ b/src/blobstore/implementations/ondisk/Data.cpp
@@ -3,8 +3,14 @@
 #include <stdexcept>
 #include <fstream>
 
+using std::istream;
 using std::ofstream;
+using std::ifstream;
 using std::ios;
+using std::unique_ptr;
+using std::make_unique;
+
+namespace bf = boost::filesystem;
 
 namespace blobstore {
 namespace ondisk {
@@ -16,6 +22,12 @@ Data::Data(size_t size)
   }
 }
 
+Data::Data(Data &&rhs)
+: _size(rhs._size), _data(rhs._data) {
+  // Make rhs invalid, so the memory doesn't get freed in its destructor.
+  rhs._data = nullptr;
+}
+
 Data::~Data() {
   std::free(_data);
   _data = nullptr;
@@ -33,10 +45,40 @@ size_t Data::size() const {
   return _size;
 }
 
-void Data::StoreToFile(const boost::filesystem::path &filepath) const {
+void Data::FillWithZeroes() {
+  std::memset(_data, 0, _size);
+}
+
+void Data::StoreToFile(const bf::path &filepath) const {
   ofstream file(filepath.c_str(), ios::binary | ios::trunc);
   file.write((const char*)_data, _size);
 }
 
+unique_ptr<Data> Data::LoadFromFile(const bf::path &filepath) {
+  ifstream file(filepath.c_str(), ios::binary);
+  size_t size = _getStreamSize(file);
+
+  auto blob = make_unique<Data>(size);
+  blob->_readFromStream(file);
+  return blob;
+}
+
+size_t Data::_getStreamSize(istream &stream) {
+  auto current_pos = stream.tellg();
+
+  //Retrieve length
+  stream.seekg(0, stream.end);
+  auto endpos = stream.tellg();
+
+  //Restore old position
+  stream.seekg(current_pos, stream.beg);
+
+  return endpos - current_pos;
+}
+
+void Data::_readFromStream(istream &stream) {
+  stream.read((char*)_data, _size);
+}
+
 } /* namespace ondisk */
 } /* namespace blobstore */
diff --git a/src/blobstore/implementations/ondisk/Data.h b/src/blobstore/implementations/ondisk/Data.h
index cc6ad787..98ebf4ef 100644
--- a/src/blobstore/implementations/ondisk/Data.h
+++ b/src/blobstore/implementations/ondisk/Data.h
@@ -7,6 +7,7 @@
 #include "fspp/utils/macros.h"
 
 #include <boost/filesystem/path.hpp>
+#include <memory>
 
 namespace blobstore {
 namespace ondisk {
@@ -14,6 +15,7 @@ namespace ondisk {
 class Data {
 public:
   Data(size_t size);
+  Data(Data &&rhs); // move constructor
   virtual ~Data();
 
   void *data();
@@ -21,12 +23,18 @@ public:
 
   size_t size() const;
 
+  void FillWithZeroes();
+
   void StoreToFile(const boost::filesystem::path &filepath) const;
+  static std::unique_ptr<Data> LoadFromFile(const boost::filesystem::path &filepath);
 
 private:
   size_t _size;
   void *_data;
 
+  static size_t _getStreamSize(std::istream &stream);
+  void _readFromStream(std::istream &stream);
+
   DISALLOW_COPY_AND_ASSIGN(Data);
 };
 
diff --git a/src/blobstore/implementations/ondisk/OnDiskBlob.cpp b/src/blobstore/implementations/ondisk/OnDiskBlob.cpp
index 52604c7f..af463891 100644
--- a/src/blobstore/implementations/ondisk/OnDiskBlob.cpp
+++ b/src/blobstore/implementations/ondisk/OnDiskBlob.cpp
@@ -21,7 +21,11 @@ namespace blobstore {
 namespace ondisk {
 
 OnDiskBlob::OnDiskBlob(const bf::path &filepath, size_t size)
- : _filepath(filepath), _size(size), _data(size) {
+ : _filepath(filepath), _data(size) {
+}
+
+OnDiskBlob::OnDiskBlob(const bf::path &filepath, Data &&data)
+ : _filepath(filepath), _data(std::move(data)) {
 }
 
 OnDiskBlob::~OnDiskBlob() {
@@ -36,41 +40,21 @@ const void *OnDiskBlob::data() const {
 }
 
 size_t OnDiskBlob::size() const {
-  return _size;
+  return _data.size();
 }
 
 unique_ptr<OnDiskBlob> OnDiskBlob::LoadFromDisk(const bf::path &filepath) {
-  ifstream file(filepath.c_str(), ios::binary);
-  size_t size = _getStreamSize(file);
+  auto data = Data::LoadFromFile(filepath);
 
-  auto blob = make_unique<OnDiskBlob>(filepath, size);
-  blob->_loadDataFromStream(file);
-  return blob;
-}
-
-size_t OnDiskBlob::_getStreamSize(istream &stream) {
-  auto current_pos = stream.tellg();
-
-  //Retrieve length
-  stream.seekg(0, stream.end);
-  auto endpos = stream.tellg();
-
-  //Restore old position
-  stream.seekg(current_pos, stream.beg);
-
-  return endpos - current_pos;
-}
-
-void OnDiskBlob::_loadDataFromStream(istream &stream) {
-  stream.read((char*)_data.data(), _size);
+  return unique_ptr<OnDiskBlob>(new OnDiskBlob(filepath, std::move(*data.get())));
 }
 
 unique_ptr<OnDiskBlob> OnDiskBlob::CreateOnDisk(const bf::path &filepath, size_t size) {
   _assertFileDoesntExist(filepath);
-  auto blob = make_unique<OnDiskBlob>(filepath, size);
+  auto blob = unique_ptr<OnDiskBlob>(new OnDiskBlob(filepath, size));
   blob->_fillDataWithZeroes();
   blob->_storeToDisk();
-  return std::move(blob);
+  return blob;
 }
 
 void OnDiskBlob::_assertFileDoesntExist(const bf::path &filepath) {
@@ -80,7 +64,7 @@ void OnDiskBlob::_assertFileDoesntExist(const bf::path &filepath) {
 }
 
 void OnDiskBlob::_fillDataWithZeroes() {
-  std::memset(_data.data(), 0, _size);
+  _data.FillWithZeroes();
 }
 
 void OnDiskBlob::_storeToDisk() const {
diff --git a/src/blobstore/implementations/ondisk/OnDiskBlob.h b/src/blobstore/implementations/ondisk/OnDiskBlob.h
index f6782296..3e81ac08 100644
--- a/src/blobstore/implementations/ondisk/OnDiskBlob.h
+++ b/src/blobstore/implementations/ondisk/OnDiskBlob.h
@@ -14,7 +14,6 @@ class OnDiskBlobStore;
 
 class OnDiskBlob: public Blob {
 public:
-  OnDiskBlob(const boost::filesystem::path &filepath, size_t size);
   virtual ~OnDiskBlob();
 
   static std::unique_ptr<OnDiskBlob> LoadFromDisk(const boost::filesystem::path &filepath);
@@ -27,13 +26,12 @@ public:
 
 private:
   const boost::filesystem::path _filepath;
-  size_t _size;
   Data _data;
 
+  OnDiskBlob(const boost::filesystem::path &filepath, size_t size);
+  OnDiskBlob(const boost::filesystem::path &filepath, Data &&data);
+
   static void _assertFileDoesntExist(const boost::filesystem::path &filepath);
-  static size_t _getStreamSize(std::istream &stream);
-  void _loadDataFromStream(std::istream &stream);
-  void _storeDataToStream(std::ostream &stream) const;
   void _fillDataWithZeroes();
   void _storeToDisk() const;
 };