Use c++11 move constructor

Use a guard to avoid complicated catch/rethrow.

10 files changed, 63 insertions, 113 deletions

diff --git a/filedistribution/src/tests/common/testCommon.cpp b/filedistribution/src/tests/common/testCommon.cpp
index 699d3628547..1902d4ec7db 100644
--- a/filedistribution/src/tests/common/testCommon.cpp
+++ b/filedistribution/src/tests/common/testCommon.cpp
@@ -12,13 +12,13 @@ namespace fd = filedistribution;
 
 const size_t bufferCapacity = 10;
 
-fd::Move<fd::Buffer>
+fd::Buffer
 getBuffer() {
     const char* test = "test";
     fd::Buffer buffer(test, test + strlen(test));
     buffer.reserve(bufferCapacity);
     buffer.push_back(0);
-    return fd::move(buffer);
+    return buffer;
 }
 
 BOOST_AUTO_TEST_CASE(bufferTest) {
diff --git a/filedistribution/src/vespa/filedistribution/common/buffer.h b/filedistribution/src/vespa/filedistribution/common/buffer.h
index 2325b3e11ee..79927119e1f 100644
--- a/filedistribution/src/vespa/filedistribution/common/buffer.h
+++ b/filedistribution/src/vespa/filedistribution/common/buffer.h
@@ -5,37 +5,6 @@
 
 namespace filedistribution {
 
-struct USED_FOR_MOVING {};
-
-template <class T>
-class Move {
-    mutable T _holder;
-public:
-    Move(T& toMove)
-        :_holder(USED_FOR_MOVING())
-    {
-        _holder.swap(toMove);
-    }
-
-    Move(const Move& other)
-        :_holder(USED_FOR_MOVING())
-    {
-        _holder.swap(other._holder);
-    }
-
-    void swap(T& t) const {
-        _holder.swap(t);
-    }
-
-private:
-    Move& operator=(const Move&);
-};
-
-template <class T>
-inline Move<T> move(T& t) {
-    return Move<T>(t);
-}
-
 class Buffer {
     size_t _capacity;
     char* _buf;
@@ -54,13 +23,15 @@ public:
          _size(0)
     {}
 
-    Buffer(const Move<Buffer>& buffer);
-
-    explicit Buffer(USED_FOR_MOVING)
-        :_capacity(0),
-         _buf(0),
-         _size(0)
-    {}
+    Buffer(Buffer && rhs) :
+        _capacity(rhs._capacity),
+        _buf(rhs._buf),
+        _size(rhs._size)
+    {
+        rhs._capacity = 0;
+        rhs._size = 0;
+        rhs._buf = nullptr;
+    }
 
     template <typename ITER>
     Buffer(ITER beginIter, ITER endIter)
@@ -75,13 +46,8 @@ public:
         delete[] _buf;
     }
 
-    size_t capacity() const {
-        return _capacity;
-    }
-
-    size_t size() const {
-        return _size;
-    }
+    size_t capacity() const { return _capacity; }
+    size_t size() const { return _size; }
 
     //might expose uninitialized memory
     void resize(size_t newSize) {
@@ -114,39 +80,14 @@ public:
         _buf[_size++] = c;
     }
 
-    iterator begin() {
-        return _buf;
-    }
-
-    iterator end() {
-        return _buf + _size;
-    }
-
-    const_iterator begin() const {
-        return _buf;
-    }
-
-    const_iterator end() const {
-        return _buf + _size;
-    }
-
-    char operator[](size_t i) const {
-        return _buf[i];
-    }
-
-    char& operator[](size_t i) {
-        return _buf[i];
-    }
+    iterator begin() { return _buf; }
+    iterator end() { return _buf + _size; }
+    const_iterator begin() const { return _buf; }
+    const_iterator end() const { return _buf + _size; }
+    char operator[](size_t i) const { return _buf[i]; }
+    char& operator[](size_t i) { return _buf[i]; }
 };
 
-inline Buffer::Buffer(const Move<Buffer>& buffer)
-    :_capacity(0),
-     _buf(0),
-     _size(0)
-{
-    buffer.swap(*this);
-}
-
 } //namespace filedistribution
 
 
diff --git a/filedistribution/src/vespa/filedistribution/manager/createtorrent.cpp b/filedistribution/src/vespa/filedistribution/manager/createtorrent.cpp
index fd54b65cdbc..a8e7505cfed 100644
--- a/filedistribution/src/vespa/filedistribution/manager/createtorrent.cpp
+++ b/filedistribution/src/vespa/filedistribution/manager/createtorrent.cpp
@@ -66,14 +66,14 @@ CreateTorrent(const boost::filesystem::path& path)
      _entry(createEntry(_path))
 {}
 
-const filedistribution::Move<filedistribution::Buffer>
+filedistribution::Buffer
 filedistribution::
 CreateTorrent::
 bencode() const
 {
     Buffer buffer(static_cast<int>(targetTorrentSize));
     libtorrent::bencode(std::back_inserter(buffer), _entry);
-    return move(buffer);
+    return buffer;
 }
 
 const std::string
diff --git a/filedistribution/src/vespa/filedistribution/manager/createtorrent.h b/filedistribution/src/vespa/filedistribution/manager/createtorrent.h
index 93d56fa9e6f..ad1ad4379dd 100644
--- a/filedistribution/src/vespa/filedistribution/manager/createtorrent.h
+++ b/filedistribution/src/vespa/filedistribution/manager/createtorrent.h
@@ -15,7 +15,7 @@ class CreateTorrent {
 public:
 
     CreateTorrent(const boost::filesystem::path& path);
-    const Move<Buffer> bencode() const;
+    Buffer bencode() const;
     const std::string fileReference() const;
 };
 
diff --git a/filedistribution/src/vespa/filedistribution/model/filedbmodel.h b/filedistribution/src/vespa/filedistribution/model/filedbmodel.h
index d7803b8afea..b575a166701 100644
--- a/filedistribution/src/vespa/filedistribution/model/filedbmodel.h
+++ b/filedistribution/src/vespa/filedistribution/model/filedbmodel.h
@@ -43,7 +43,7 @@ public:
 
     virtual bool hasFile(const std::string& fileReference) = 0;
     virtual void addFile(const std::string& fileReference, const Buffer& buffer) = 0;
-    virtual Move<Buffer> getFile(const std::string& fileReference) = 0;
+    virtual Buffer getFile(const std::string& fileReference) = 0;
     virtual void cleanFiles(const std::vector<std::string>& filesToPreserve) = 0;
 
     virtual void setDeployedFilesToDownload(const std::string& hostName,
diff --git a/filedistribution/src/vespa/filedistribution/model/mockfiledistributionmodel.h b/filedistribution/src/vespa/filedistribution/model/mockfiledistributionmodel.h
index 169225deaf2..cdc77692017 100644
--- a/filedistribution/src/vespa/filedistribution/model/mockfiledistributionmodel.h
+++ b/filedistribution/src/vespa/filedistribution/model/mockfiledistributionmodel.h
@@ -21,15 +21,14 @@ public:
         _fileReferences.push_back(fileReference);
     }
 
-    Move<Buffer> getFile(const std::string& fileReference) {
+    Buffer getFile(const std::string& fileReference) override {
         (void)fileReference;
         const char* resultStr = "result";
         Buffer result(resultStr, resultStr + strlen(resultStr));
-        return move(result);
+        return result;
     }
 
-    virtual void cleanFiles(
-            const std::vector<std::string> &) {}
+    virtual void cleanFiles(const std::vector<std::string> &) {}
 
 
     virtual void setDeployedFilesToDownload(const std::string&,
@@ -50,8 +49,7 @@ public:
         return HostStatus();
     }
 
-    Progress getProgress(const std::string&,
-                         const std::vector<std::string>&) {
+    Progress getProgress(const std::string&, const std::vector<std::string>&) {
         return Progress();
     }
 };
diff --git a/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp b/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp
index 4e93b0907ab..89ff72e4a36 100644
--- a/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp
+++ b/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp
@@ -21,7 +21,6 @@
 typedef std::unique_lock<std::mutex> UniqueLock;
 
 using filedistribution::ZKFacade;
-using filedistribution::Move;
 using filedistribution::Buffer;
 using filedistribution::ZKGenericException;
 using filedistribution::ZKException;
@@ -333,7 +332,7 @@ ZKFacade::getString(const Path& path) {
     return std::string(buffer.begin(), buffer.end());
 }
 
-const Move<Buffer>
+Buffer
 ZKFacade::getData(const Path& path) {
     RetryController retryController(this);
     Buffer buffer(_maxDataSize);
@@ -349,34 +348,30 @@ ZKFacade::getData(const Path& path) {
 
     retryController.throwIfError(path);
     buffer.resize(bufferSize);
-    return move(buffer);
+    return buffer;
 }
 
-const Move<Buffer>
+Buffer
 ZKFacade::getData(const Path& path, const NodeChangedWatcherSP& watcher) {
-    void* watcherContext = registerWatcher(watcher);
+    RegistrationGuard unregisterGuard(*this, watcher);
+    void* watcherContext = unregisterGuard.get();
     RetryController retryController(this);
 
-    try {
-        Buffer buffer(_maxDataSize);
-        int bufferSize = _maxDataSize;
-
-        do {
-            Stat stat;
-            bufferSize = _maxDataSize;
+    Buffer buffer(_maxDataSize);
+    int bufferSize = _maxDataSize;
 
-            retryController( zoo_wget(_zhandle, path.string().c_str(), &ZKWatcher::watcherFn, watcherContext, &*buffer.begin(), &bufferSize, &stat));
-        } while(retryController.shouldRetry());
+    do {
+        Stat stat;
+        bufferSize = _maxDataSize;
 
-        retryController.throwIfError(path);
+        retryController( zoo_wget(_zhandle, path.string().c_str(), &ZKWatcher::watcherFn, watcherContext, &*buffer.begin(), &bufferSize, &stat));
+    } while (retryController.shouldRetry());
 
-        buffer.resize(bufferSize);
-        return move(buffer);
+    retryController.throwIfError(path);
 
-    } catch (const ZKException & e) {
-        unregisterWatcher(watcherContext);
-        throw;
-    }
+    buffer.resize(bufferSize);
+    unregisterGuard.release();
+    return buffer;
 }
 
 void
diff --git a/filedistribution/src/vespa/filedistribution/model/zkfacade.h b/filedistribution/src/vespa/filedistribution/model/zkfacade.h
index b6a3e42d899..7e6db4d381f 100644
--- a/filedistribution/src/vespa/filedistribution/model/zkfacade.h
+++ b/filedistribution/src/vespa/filedistribution/model/zkfacade.h
@@ -90,9 +90,9 @@ public:
     bool hasNode(const Path&, const NodeChangedWatcherSP&);
 
     const std::string getString(const Path&);
-    const Move<Buffer> getData(const Path&);  //throws ZKNodeDoesNotExistsException
+    Buffer getData(const Path&);  //throws ZKNodeDoesNotExistsException
     //if watcher is specified, it will be set even if the node does not exists
-    const Move<Buffer> getData(const Path&, const NodeChangedWatcherSP&);  //throws ZKNodeDoesNotExistsException
+    Buffer getData(const Path&, const NodeChangedWatcherSP&);  //throws ZKNodeDoesNotExistsException
 
     //Parent path must exist
     void setData(const Path&, const Buffer& buffer, bool mustExist = false);
@@ -116,6 +116,22 @@ public:
     }
 
 private:
+    class RegistrationGuard {
+    public:
+        RegistrationGuard & operator = (const RegistrationGuard &) = delete;
+        RegistrationGuard(const RegistrationGuard &) = delete;
+        RegistrationGuard(ZKFacade & zk, const NodeChangedWatcherSP & watcher) : _zk(zk), _watcherContext(_zk.registerWatcher(watcher)) { }
+        ~RegistrationGuard() {
+            if (_watcherContext) {
+                _zk.unregisterWatcher(_watcherContext);
+            }
+        }
+        void * get() { return _watcherContext; }
+        void release() { _watcherContext = nullptr; }
+    private:
+        ZKFacade & _zk;
+        void     * _watcherContext;
+    };
     void* registerWatcher(const NodeChangedWatcherSP &); //returns watcherContext
     std::shared_ptr<ZKWatcher> unregisterWatcher(void* watcherContext);
     void invokeWatcher(void* watcherContext);
diff --git a/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp b/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp
index 70827305138..ab886a5889b 100644
--- a/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp
+++ b/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp
@@ -58,7 +58,7 @@ ZKFileDBModel::addFile(const std::string& fileReference, const Buffer& buffer) {
     return _zk->setData(createPath(fileReference), buffer);
 }
 
-Move<Buffer>
+Buffer
 ZKFileDBModel::getFile(const std::string& fileReference) {
     try {
         return _zk->getData(createPath(fileReference));
diff --git a/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.h b/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.h
index 4249410c00e..fc3b088a385 100644
--- a/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.h
+++ b/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.h
@@ -29,7 +29,7 @@ public:
     //overrides
     bool hasFile(const std::string& fileReference);
     void addFile(const std::string& fileReference, const Buffer& buffer);
-    Move<Buffer> getFile(const std::string& fileReference);
+    Buffer getFile(const std::string& fileReference) override;
     void cleanFiles(const std::vector<std::string>& filesToPreserve);
 
     void setDeployedFilesToDownload(const std::string& hostName,