Improve Buffer and Slice:

* Make Buffer constructible from a Slice
* Fix self-move-assignment in Buffer
* Add unit tests

Builds/VisualStudio2015/RippleD.vcxproj

@@ -4414,6 +4414,10 @@
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
     </ClCompile>
+    <ClCompile Include="..\..\src\test\basics\Buffer_test.cpp">
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
+    </ClCompile>
     <ClCompile Include="..\..\src\test\basics\CheckLibraryVersions_test.cpp">
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>

Builds/VisualStudio2015/RippleD.vcxproj.filters

@@ -5205,6 +5205,9 @@
     <ClCompile Include="..\..\src\test\basics\base_uint_test.cpp">
       <Filter>test\basics</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\src\test\basics\Buffer_test.cpp">
+      <Filter>test\basics</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\src\test\basics\CheckLibraryVersions_test.cpp">
       <Filter>test\basics</Filter>
     </ClCompile>

src/ripple/basics/Buffer.h

@@ -21,6 +21,7 @@
 #define RIPPLE_BASICS_BUFFER_H_INCLUDED
 
 #include <ripple/basics/Slice.h>
+#include <cassert>
 #include <cstdint>
 #include <cstring>
 #include <memory>
@@ -34,14 +35,49 @@ namespace ripple {
 class Buffer
 {
 private:
-    std::unique_ptr<
+    std::unique_ptr<std::uint8_t[]> p_;
-        std::uint8_t[]> p_;
     std::size_t size_ = 0;
 
 public:
     Buffer() = default;
-    Buffer (Buffer const&) = delete;
+
-    Buffer& operator= (Buffer const&) = delete;
+    /** Create an uninitialized buffer with the given size. */
+    explicit
+    Buffer (std::size_t size)
+        : p_ (size ? new std::uint8_t[size] : nullptr)
+        , size_ (size)
+    {
+    }
+
+    /** Create a buffer as a copy of existing memory.
+
+        @param data a pointer to the existing memory. If
+                    size is non-zero, it must not be null.
+        @param size size of the existing memory block.
+    */
+    Buffer (void const* data, std::size_t size)
+        : Buffer (size)
+    {
+        if (size)
+            std::memcpy(p_.get(), data, size);
+    }
+
+    /** Copy-construct */
+    Buffer (Buffer const& other)
+        : Buffer (other.p_.get(), other.size_)
+    {
+    }
+
+    /** Copy assign */
+    Buffer& operator= (Buffer const& other)
+    {
+        if (this != &other)
+        {
+            if (auto p = alloc (other.size_))
+                std::memcpy (p, other.p_.get(), size_);
+        }
+        return *this;
+    }
 
     /** Move-construct.
         The other buffer is reset.
@@ -57,34 +93,34 @@ public:
         The other buffer is reset.
     */
     Buffer& operator= (Buffer&& other)
+    {
+        if (this != &other)
         {
             p_ = std::move(other.p_);
             size_ = other.size_;
             other.size_ = 0;
+        }
         return *this;
     }
 
-    /** Create an uninitialized buffer with the given size. */
+    /** Construct from a slice */
     explicit
-    Buffer (std::size_t size)
+    Buffer (Slice s)
-        : p_ (size ?
+        : Buffer (s.data(), s.size())
-            new std::uint8_t[size] : nullptr)
-        , size_ (size)
     {
     }
 
-    /** Create a buffer as a copy of existing memory. */
+    /** Assign from slice */
-    Buffer (void const* data, std::size_t size)
+    Buffer& operator= (Slice s)
-        : Buffer (size)
     {
-        std::memcpy(p_.get(), data, size);
+        // Ensure the slice isn't a subset of the buffer.
-    }
+        assert (s.size() == 0 || size_ == 0 ||
+            s.data() < p_.get() ||
+            s.data() >= p_.get() + size_);
 
-    /** Create a buffer from a copy of existing memory. */
+        if (auto p = alloc (s.size()))
-    explicit
+            std::memcpy (p, s.data(), s.size());
-    Buffer (Slice const& slice)
+        return *this;
-        : Buffer(slice.data(), slice.size())
-    {
     }
 
     /** Returns the number of bytes in the buffer. */
@@ -141,14 +177,9 @@ public:
     std::uint8_t*
     alloc (std::size_t n)
     {
-        if (n == 0)
-        {
-            clear();
-            return nullptr;
-        }
         if (n != size_)
         {
-            p_.reset(new std::uint8_t[n]);
+            p_.reset(n ? new std::uint8_t[n] : nullptr);
             size_ = n;
         }
         return p_.get();

src/test/basics/Buffer_test.cpp

@@ -0,0 +1,274 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github0.com/ripple/rippled
+    Copyright (c) 2012-2016 Ripple Labs Inc.
+
+    Permission to use, copy, modify, and/or distribute this software for any
+    purpose  with  or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+#include <BeastConfig.h>
+#include <ripple/basics/Buffer.h>
+#include <ripple/beast/unit_test.h>
+#include <cstdint>
+
+namespace ripple {
+namespace test {
+
+struct Buffer_test : beast::unit_test::suite
+{
+    bool sane (Buffer const& b) const
+    {
+        if (b.size() == 0)
+            return b.data() == nullptr;
+
+        return b.data() != nullptr;
+    }
+
+    void run()
+    {
+        std::uint8_t const data[] =
+        {
+            0xa8, 0xa1, 0x38, 0x45, 0x23, 0xec, 0xe4, 0x23,
+            0x71, 0x6d, 0x2a, 0x18, 0xb4, 0x70, 0xcb, 0xf5,
+            0xac, 0x2d, 0x89, 0x4d, 0x19, 0x9c, 0xf0, 0x2c,
+            0x15, 0xd1, 0xf9, 0x9b, 0x66, 0xd2, 0x30, 0xd3
+        };
+
+        Buffer b0;
+        BEAST_EXPECT (sane (b0));
+        BEAST_EXPECT (b0.empty());
+
+        Buffer b1 { 0 };
+        BEAST_EXPECT (sane (b1));
+        BEAST_EXPECT (b1.empty());
+        std::memcpy(b1.alloc(16), data, 16);
+        BEAST_EXPECT (sane (b1));
+        BEAST_EXPECT (!b1.empty());
+        BEAST_EXPECT (b1.size() == 16);
+
+        Buffer b2 { b1.size() };
+        BEAST_EXPECT (sane (b2));
+        BEAST_EXPECT (!b2.empty());
+        BEAST_EXPECT (b2.size() == b1.size());
+        std::memcpy(b2.data(), data + 16, 16);
+
+        Buffer b3 { data, sizeof(data) };
+        BEAST_EXPECT (sane (b3));
+        BEAST_EXPECT (!b3.empty());
+        BEAST_EXPECT (b3.size() == sizeof(data));
+        BEAST_EXPECT (std::memcmp (b3.data(), data, b3.size()) == 0);
+
+        // Check equality and inequality comparisons
+        BEAST_EXPECT (b0 == b0);
+        BEAST_EXPECT (b0 != b1);
+        BEAST_EXPECT (b1 == b1);
+        BEAST_EXPECT (b1 != b2);
+        BEAST_EXPECT (b2 != b3);
+
+        // Check copy constructors and copy assignments:
+        {
+            testcase ("Copy Construction / Assignment");
+
+            Buffer x{ b0 };
+            BEAST_EXPECT (x == b0);
+            BEAST_EXPECT (sane (x));
+            Buffer y{ b1 };
+            BEAST_EXPECT (y == b1);
+            BEAST_EXPECT (sane (y));
+            x = b2;
+            BEAST_EXPECT (x == b2);
+            BEAST_EXPECT (sane (x));
+            x = y;
+            BEAST_EXPECT (x == y);
+            BEAST_EXPECT (sane (x));
+            y = b3;
+            BEAST_EXPECT (y == b3);
+            BEAST_EXPECT (sane (y));
+            x = b0;
+            BEAST_EXPECT (x == b0);
+            BEAST_EXPECT (sane (x));
+            x = x;
+            BEAST_EXPECT (x == b0);
+            BEAST_EXPECT (sane (x));
+            y = y;
+            BEAST_EXPECT (y == b3);
+            BEAST_EXPECT (sane (y));
+        }
+
+        // Check move constructor & move assignments:
+        {
+            testcase ("Move Construction / Assignment");
+
+            { // Move-construct from empty buf
+                Buffer x;
+                Buffer y { std::move(x) };
+                BEAST_EXPECT (sane(x));
+                BEAST_EXPECT (x.empty());
+                BEAST_EXPECT (sane(y));
+                BEAST_EXPECT (y.empty());
+                BEAST_EXPECT (x == y);
+            }
+
+            { // Move-construct from non-empty buf
+                Buffer x { b1 };
+                Buffer y { std::move(x) };
+                BEAST_EXPECT (sane(x));
+                BEAST_EXPECT (x.empty());
+                BEAST_EXPECT (sane(y));
+                BEAST_EXPECT (y == b1);
+            }
+
+            { // Move assign empty buf to empty buf
+                Buffer x;
+                Buffer y;
+
+                x = std::move(y);
+                BEAST_EXPECT (sane(x));
+                BEAST_EXPECT (x.empty());
+                BEAST_EXPECT (sane(y));
+                BEAST_EXPECT (y.empty());
+            }
+
+            { // Move assign non-empty buf to empty buf
+                Buffer x;
+                Buffer y { b1 };
+
+                x = std::move(y);
+                BEAST_EXPECT (sane(x));
+                BEAST_EXPECT (x == b1);
+                BEAST_EXPECT (sane(y));
+                BEAST_EXPECT (y.empty());
+            }
+
+            { // Move assign empty buf to non-empty buf
+                Buffer x { b1 };
+                Buffer y;
+
+                x = std::move(y);
+                BEAST_EXPECT (sane(x));
+                BEAST_EXPECT (x.empty());
+                BEAST_EXPECT (sane(y));
+                BEAST_EXPECT (y.empty());
+            }
+
+            { // Move assign non-empty buf to non-empty buf
+                Buffer x { b1 };
+                Buffer y { b2 };
+                Buffer z { b3 };
+
+                x = std::move(y);
+                BEAST_EXPECT (sane(x));
+                BEAST_EXPECT (!x.empty());
+                BEAST_EXPECT (sane(y));
+                BEAST_EXPECT (y.empty());
+
+                x = std::move(z);
+                BEAST_EXPECT (sane(x));
+                BEAST_EXPECT (!x.empty());
+                BEAST_EXPECT (sane(z));
+                BEAST_EXPECT (z.empty());
+            }
+        }
+
+        {
+            testcase ("Slice Conversion / Construction / Assignment");
+
+            Buffer w { static_cast<Slice>(b0) };
+            BEAST_EXPECT(sane(w));
+            BEAST_EXPECT(w == b0);
+
+            Buffer x { static_cast<Slice>(b1) };
+            BEAST_EXPECT(sane(x));
+            BEAST_EXPECT(x == b1);
+
+            Buffer y { static_cast<Slice>(b2) };
+            BEAST_EXPECT(sane(y));
+            BEAST_EXPECT(y == b2);
+
+            Buffer z { static_cast<Slice>(b3) };
+            BEAST_EXPECT(sane(z));
+            BEAST_EXPECT(z == b3);
+
+            // Assign empty slice to empty buffer
+            w = static_cast<Slice>(b0);
+            BEAST_EXPECT(sane(w));
+            BEAST_EXPECT(w == b0);
+
+            // Assign non-empty slice to empty buffer
+            w = static_cast<Slice>(b1);
+            BEAST_EXPECT(sane(w));
+            BEAST_EXPECT(w == b1);
+
+            // Assign non-empty slice to non-empty buffer
+            x = static_cast<Slice>(b2);
+            BEAST_EXPECT(sane(x));
+            BEAST_EXPECT(x == b2);
+
+            // Assign non-empty slice to non-empty buffer
+            y = static_cast<Slice>(z);
+            BEAST_EXPECT(sane(y));
+            BEAST_EXPECT(y == z);
+
+            // Assign empty slice to non-empty buffer:
+            z = static_cast<Slice>(b0);
+            BEAST_EXPECT(sane(z));
+            BEAST_EXPECT (z == b0);
+        }
+
+        {
+            testcase ("Allocation, Deallocation and Clearing");
+
+            auto test = [this](Buffer const& b, std::size_t i)
+            {
+                Buffer x{b};
+
+                // Try to allocate some number of bytes, possibly
+                // zero (which means clear) and sanity check
+                x(i);
+                BEAST_EXPECT (sane(x));
+                BEAST_EXPECT (x.size() == i);
+                BEAST_EXPECT ((x.data() == nullptr) == (i == 0));
+
+                // Try to allocate some more data (always non-zero)
+                x(i + 1);
+                BEAST_EXPECT (sane(x));
+                BEAST_EXPECT (x.size() == i + 1);
+                BEAST_EXPECT (x.data() != nullptr);
+
+                // Try to clear:
+                x.clear();
+                BEAST_EXPECT (sane(x));
+                BEAST_EXPECT (x.size() == 0);
+                BEAST_EXPECT (x.data() == nullptr);
+
+                // Try to clear again:
+                x.clear();
+                BEAST_EXPECT (sane(x));
+                BEAST_EXPECT (x.size() == 0);
+                BEAST_EXPECT (x.data() == nullptr);
+            };
+
+            for (std::size_t i = 0; i < 16; ++i)
+            {
+                test (b0, i);
+                test (b1, i);
+            }
+        }
+    }
+};
+
+BEAST_DEFINE_TESTSUITE(Buffer, ripple_basics, ripple);
+
+}  // namespace test
+}  // namespace ripple

src/unity/basics_test_unity.cpp

@@ -19,6 +19,7 @@
 //==============================================================================
 
 #include <test/basics/base_uint_test.cpp>
+#include <test/basics/Buffer_test.cpp>
 #include <test/basics/CheckLibraryVersions_test.cpp>
 #include <test/basics/contract_test.cpp>
 #include <test/basics/hardened_hash_test.cpp>