From 4b0d8b630c71215a8721685c685f76c1124df246 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Mon, 11 Jul 2016 23:46:53 -0700 Subject: [PATCH] Improve Buffer and Slice: * Make Buffer constructible from a Slice * Fix self-move-assignment in Buffer * Add unit tests --- Builds/VisualStudio2015/RippleD.vcxproj | 4 + .../VisualStudio2015/RippleD.vcxproj.filters | 3 + src/ripple/basics/Buffer.h | 87 ++++-- src/test/basics/Buffer_test.cpp | 274 ++++++++++++++++++ src/unity/basics_test_unity.cpp | 3 +- 5 files changed, 342 insertions(+), 29 deletions(-) create mode 100644 src/test/basics/Buffer_test.cpp diff --git a/Builds/VisualStudio2015/RippleD.vcxproj b/Builds/VisualStudio2015/RippleD.vcxproj index 3dd0a7428f..78c9ae19ca 100644 --- a/Builds/VisualStudio2015/RippleD.vcxproj +++ b/Builds/VisualStudio2015/RippleD.vcxproj @@ -4414,6 +4414,10 @@ True True + + True + True + True True diff --git a/Builds/VisualStudio2015/RippleD.vcxproj.filters b/Builds/VisualStudio2015/RippleD.vcxproj.filters index 6b976dcbae..dd3d32805e 100644 --- a/Builds/VisualStudio2015/RippleD.vcxproj.filters +++ b/Builds/VisualStudio2015/RippleD.vcxproj.filters @@ -5205,6 +5205,9 @@ test\basics + + test\basics + test\basics diff --git a/src/ripple/basics/Buffer.h b/src/ripple/basics/Buffer.h index f60ad296f0..3e9bf37037 100644 --- a/src/ripple/basics/Buffer.h +++ b/src/ripple/basics/Buffer.h @@ -21,6 +21,7 @@ #define RIPPLE_BASICS_BUFFER_H_INCLUDED #include +#include #include #include #include @@ -34,14 +35,49 @@ namespace ripple { class Buffer { private: - std::unique_ptr< - std::uint8_t[]> p_; + std::unique_ptr 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. @@ -58,33 +94,33 @@ public: */ Buffer& operator= (Buffer&& other) { - p_ = std::move(other.p_); - size_ = other.size_; - other.size_ = 0; + 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) - : p_ (size ? - new std::uint8_t[size] : nullptr) - , size_ (size) + Buffer (Slice s) + : Buffer (s.data(), s.size()) { } - /** Create a buffer as a copy of existing memory. */ - Buffer (void const* data, std::size_t size) - : Buffer (size) + /** Assign from slice */ + Buffer& operator= (Slice s) { - 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. */ - explicit - Buffer (Slice const& slice) - : Buffer(slice.data(), slice.size()) - { + if (auto p = alloc (s.size())) + std::memcpy (p, s.data(), s.size()); + return *this; } /** 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(); diff --git a/src/test/basics/Buffer_test.cpp b/src/test/basics/Buffer_test.cpp new file mode 100644 index 0000000000..9a29d0d018 --- /dev/null +++ b/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 +#include +#include +#include + +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(b0) }; + BEAST_EXPECT(sane(w)); + BEAST_EXPECT(w == b0); + + Buffer x { static_cast(b1) }; + BEAST_EXPECT(sane(x)); + BEAST_EXPECT(x == b1); + + Buffer y { static_cast(b2) }; + BEAST_EXPECT(sane(y)); + BEAST_EXPECT(y == b2); + + Buffer z { static_cast(b3) }; + BEAST_EXPECT(sane(z)); + BEAST_EXPECT(z == b3); + + // Assign empty slice to empty buffer + w = static_cast(b0); + BEAST_EXPECT(sane(w)); + BEAST_EXPECT(w == b0); + + // Assign non-empty slice to empty buffer + w = static_cast(b1); + BEAST_EXPECT(sane(w)); + BEAST_EXPECT(w == b1); + + // Assign non-empty slice to non-empty buffer + x = static_cast(b2); + BEAST_EXPECT(sane(x)); + BEAST_EXPECT(x == b2); + + // Assign non-empty slice to non-empty buffer + y = static_cast(z); + BEAST_EXPECT(sane(y)); + BEAST_EXPECT(y == z); + + // Assign empty slice to non-empty buffer: + z = static_cast(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 diff --git a/src/unity/basics_test_unity.cpp b/src/unity/basics_test_unity.cpp index 1a4a13d1f1..22190c4023 100644 --- a/src/unity/basics_test_unity.cpp +++ b/src/unity/basics_test_unity.cpp @@ -19,6 +19,7 @@ //============================================================================== #include +#include #include #include #include @@ -26,4 +27,4 @@ #include #include #include -#include \ No newline at end of file +#include