From 321af29d2578b1966235e4ef92c020865550fcea Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Sun, 16 Dec 2018 15:07:16 -0800 Subject: [PATCH] Refactor static_buffer: * Improve tests and coverage * Add static_buffer_base::clear * Use BOOST_BEAST_DECL --- CHANGELOG.md | 1 + .../boost/beast/core/detail/buffers_pair.hpp | 1 + .../boost/beast/core/flat_static_buffer.hpp | 13 +- .../boost/beast/core/impl/static_buffer.hpp | 22 +- include/boost/beast/core/static_buffer.hpp | 62 +++-- test/beast/core/static_buffer.cpp | 212 +++++------------- 6 files changed, 106 insertions(+), 205 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5511df80..c606e3d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ Version 200 * Add more tests for dynamic buffers * Tidy up multi_buffer * Refactor ostream +* Refactor static_buffer API Changes: diff --git a/include/boost/beast/core/detail/buffers_pair.hpp b/include/boost/beast/core/detail/buffers_pair.hpp index 79033629..6fe4b385 100644 --- a/include/boost/beast/core/detail/buffers_pair.hpp +++ b/include/boost/beast/core/detail/buffers_pair.hpp @@ -78,6 +78,7 @@ public: { b_[0] = *other.begin(); b_[1] = *(other.begin() + 1); + return *this; } buffers_pair(value_type b0, value_type b1) diff --git a/include/boost/beast/core/flat_static_buffer.hpp b/include/boost/beast/core/flat_static_buffer.hpp index 954080c9..87c0d423 100644 --- a/include/boost/beast/core/flat_static_buffer.hpp +++ b/include/boost/beast/core/flat_static_buffer.hpp @@ -76,7 +76,18 @@ public: reset(p, n); } - /// Change the number of readable and writable bytes to zero. + /** Clear the readable and writable bytes to zero. + + This function causes the readable and writable bytes + to become empty. The capacity is not changed. + + Buffer sequences previously obtained using @ref data or + @ref prepare become invalid. + + @par Exception Safety + + No-throw guarantee. + */ BOOST_BEAST_DECL void clear() noexcept; diff --git a/include/boost/beast/core/impl/static_buffer.hpp b/include/boost/beast/core/impl/static_buffer.hpp index fbc2cfc9..ead712b4 100644 --- a/include/boost/beast/core/impl/static_buffer.hpp +++ b/include/boost/beast/core/impl/static_buffer.hpp @@ -29,6 +29,15 @@ static_buffer_base( { } +void +static_buffer_base:: +clear() noexcept +{ + in_off_ = 0; + in_size_ = 0; + out_size_ = 0; +} + auto static_buffer_base:: data() const noexcept -> @@ -73,7 +82,7 @@ prepare(std::size_t n) -> using net::mutable_buffer; if(n > capacity_ - in_size_) BOOST_THROW_EXCEPTION(std::length_error{ - "buffer overflow"}); + "static_buffer overflow"}); out_size_ = n; auto const out_off = (in_off_ + in_size_) % capacity_; @@ -117,17 +126,6 @@ consume(std::size_t n) noexcept } } -void -static_buffer_base:: -reset(void* p, std::size_t n) noexcept -{ - begin_ = static_cast(p); - capacity_ = n; - in_off_ = 0; - in_size_ = 0; - out_size_ = 0; -} - //------------------------------------------------------------------------------ template diff --git a/include/boost/beast/core/static_buffer.hpp b/include/boost/beast/core/static_buffer.hpp index 62d35676..204e6a37 100644 --- a/include/boost/beast/core/static_buffer.hpp +++ b/include/boost/beast/core/static_buffer.hpp @@ -73,9 +73,25 @@ public: @param size The number of valid bytes pointed to by `p`. */ - inline + BOOST_BEAST_DECL static_buffer_base(void* p, std::size_t size) noexcept; + /** Clear the readable and writable bytes to zero. + + This function causes the readable and writable bytes + to become empty. The capacity is not changed. + + Buffer sequences previously obtained using @ref data or + @ref prepare become invalid. + + @par Exception Safety + + No-throw guarantee. + */ + BOOST_BEAST_DECL + void + clear() noexcept; + //-------------------------------------------------------------------------- #if BOOST_BEAST_DOXYGEN @@ -115,7 +131,7 @@ public: } /// Returns a constant buffer sequence representing the readable bytes - inline + BOOST_BEAST_DECL const_buffers_type data() const noexcept; @@ -127,7 +143,7 @@ public: } /// Returns a mutable buffer sequence representing the readable bytes - inline + BOOST_BEAST_DECL mutable_data_type data() noexcept; @@ -149,7 +165,7 @@ public: Strong guarantee. */ - inline + BOOST_BEAST_DECL mutable_buffers_type prepare(std::size_t n); @@ -171,7 +187,7 @@ public: No-throw guarantee. */ - inline + BOOST_BEAST_DECL void commit(std::size_t n) noexcept; @@ -190,37 +206,9 @@ public: No-throw guarantee. */ - inline + BOOST_BEAST_DECL void consume(std::size_t n) noexcept; - -protected: - /** Constructor - - The buffer will be in an undefined state. It is necessary - for the derived class to call @ref reset in order to - initialize the object. - */ - static_buffer_base() = default; - - /** Reset the pointed-to buffer. - - This function resets the internal state to the buffer provided. - All input and output sequences are invalidated. This function - allows the derived class to construct its members before - initializing the static buffer. - - @param p A pointer to valid storage of at least `n` bytes. - - @param size The number of valid bytes pointed to by `p`. - - @par Exception Safety - - No-throw guarantee. - */ - inline - void - reset(void* p, std::size_t size) noexcept; }; //------------------------------------------------------------------------------ @@ -261,15 +249,15 @@ class static_buffer : public static_buffer_base char buf_[N]; public: - /// Constructor - static_buffer(static_buffer const&) noexcept; - /// Constructor static_buffer() noexcept : static_buffer_base(buf_, N) { } + /// Constructor + static_buffer(static_buffer const&) noexcept; + /// Assignment static_buffer& operator=(static_buffer const&) noexcept; diff --git a/test/beast/core/static_buffer.cpp b/test/beast/core/static_buffer.cpp index dc83df27..c54043cd 100644 --- a/test/beast/core/static_buffer.cpp +++ b/test/beast/core/static_buffer.cpp @@ -24,27 +24,17 @@ namespace boost { namespace beast { -BOOST_STATIC_ASSERT( - net::is_dynamic_buffer::value); - class static_buffer_test : public beast::unit_test::suite { public: BOOST_STATIC_ASSERT( - net::is_dynamic_buffer< + is_mutable_dynamic_buffer< + static_buffer<13>>::value); + + BOOST_STATIC_ASSERT( + is_mutable_dynamic_buffer< static_buffer_base>::value); - BOOST_STATIC_ASSERT( - net::is_const_buffer_sequence< - static_buffer_base::const_buffers_type>::value); - BOOST_STATIC_ASSERT( - net::is_mutable_buffer_sequence< - static_buffer_base::mutable_data_type>::value); - BOOST_STATIC_ASSERT( - net::is_mutable_buffer_sequence< - static_buffer_base::mutable_buffers_type>::value); - BOOST_STATIC_ASSERT(std::is_convertible< - static_buffer_base::mutable_data_type, - static_buffer_base::const_buffers_type>::value); + #if ! defined( BOOST_LIBSTDCXX_VERSION ) || BOOST_LIBSTDCXX_VERSION >= 50000 # ifndef BOOST_ASIO_ENABLE_BUFFER_DEBUGGING BOOST_STATIC_ASSERT(std::is_trivially_copyable< @@ -54,147 +44,18 @@ public: # endif #endif - template void - testMutableData() + testDynamicBuffer() { - DynamicBuffer b; - DynamicBuffer const& cb = b; - ostream(b) << "Hello"; - BOOST_STATIC_ASSERT( - net::is_const_buffer_sequence< - decltype(cb.data())>::value && - ! net::is_mutable_buffer_sequence< - decltype(cb.data())>::value); - BOOST_STATIC_ASSERT( - net::is_const_buffer_sequence< - decltype(cb.cdata())>::value && - ! net::is_mutable_buffer_sequence< - decltype(cb.cdata())>::value); - BOOST_STATIC_ASSERT( - net::is_mutable_buffer_sequence< - decltype(b.data())>::value); - std::for_each( - net::buffers_iterator::begin(b.data()), - net::buffers_iterator::end(b.data()), - [](char& c) - { - c = static_cast(std::toupper(c)); - }); - BEAST_EXPECT(buffers_to_string(b.data()) == "HELLO"); - BEAST_EXPECT(buffers_to_string(b.cdata()) == "HELLO"); + static_buffer<13> b; + test_dynamic_buffer(*this, b); } void - testStaticBuffer() + testMembers() { - using net::buffer; using net::buffer_size; - char buf[12]; - std::string const s = "Hello, world"; - BEAST_EXPECT(s.size() == sizeof(buf)); - for(std::size_t i = 1; i < 4; ++i) { - for(std::size_t j = 1; j < 4; ++j) { - for(std::size_t x = 1; x < 4; ++x) { - for(std::size_t y = 1; y < 4; ++y) { - for(std::size_t t = 1; t < 4; ++ t) { - for(std::size_t u = 1; u < 4; ++ u) { - std::size_t z = sizeof(buf) - (x + y); - std::size_t v = sizeof(buf) - (t + u); - { - std::memset(buf, 0, sizeof(buf)); - static_buffer ba; - { - auto d = ba.prepare(z); - BEAST_EXPECT(buffer_size(d) == z); - } - { - auto d = ba.prepare(0); - BEAST_EXPECT(buffer_size(d) == 0); - } - { - auto d = ba.prepare(y); - BEAST_EXPECT(buffer_size(d) == y); - } - { - auto d = ba.prepare(x); - BEAST_EXPECT(buffer_size(d) == x); - ba.commit(buffer_copy(d, buffer(s.data(), x))); - } - BEAST_EXPECT(ba.size() == x); - BEAST_EXPECT(buffer_size(ba.data()) == ba.size()); - { - auto d = ba.prepare(x); - BEAST_EXPECT(buffer_size(d) == x); - } - { - auto d = ba.prepare(0); - BEAST_EXPECT(buffer_size(d) == 0); - } - { - auto d = ba.prepare(z); - BEAST_EXPECT(buffer_size(d) == z); - } - { - auto d = ba.prepare(y); - BEAST_EXPECT(buffer_size(d) == y); - ba.commit(buffer_copy(d, buffer(s.data()+x, y))); - } - ba.commit(1); - BEAST_EXPECT(ba.size() == x + y); - BEAST_EXPECT(buffer_size(ba.data()) == ba.size()); - { - auto d = ba.prepare(x); - BEAST_EXPECT(buffer_size(d) == x); - } - { - auto d = ba.prepare(y); - BEAST_EXPECT(buffer_size(d) == y); - } - { - auto d = ba.prepare(0); - BEAST_EXPECT(buffer_size(d) == 0); - } - { - auto d = ba.prepare(z); - BEAST_EXPECT(buffer_size(d) == z); - ba.commit(buffer_copy(d, buffer(s.data()+x+y, z))); - } - ba.commit(2); - BEAST_EXPECT(ba.size() == x + y + z); - BEAST_EXPECT(buffer_size(ba.data()) == ba.size()); - BEAST_EXPECT(buffers_to_string(ba.data()) == s); - ba.consume(t); - { - auto d = ba.prepare(0); - BEAST_EXPECT(buffer_size(d) == 0); - } - BEAST_EXPECT(buffers_to_string(ba.data()) == s.substr(t, std::string::npos)); - ba.consume(u); - BEAST_EXPECT(buffers_to_string(ba.data()) == s.substr(t + u, std::string::npos)); - ba.consume(v); - BEAST_EXPECT(buffers_to_string(ba.data()) == ""); - ba.consume(1); - { - auto d = ba.prepare(0); - BEAST_EXPECT(buffer_size(d) == 0); - } - try - { - ba.prepare(ba.capacity() - ba.size() + 1); - fail(); - } - catch(...) - { - pass(); - } - } - }}}}}} - } - void - testBuffer() - { string_view const s = "Hello, world!"; // static_buffer_base @@ -203,8 +64,9 @@ public: static_buffer_base b{buf, sizeof(buf)}; ostream(b) << s; BEAST_EXPECT(buffers_to_string(b.data()) == s); - b.consume(b.size()); - BEAST_EXPECT(buffers_to_string(b.data()) == ""); + b.clear(); + BEAST_EXPECT(b.size() == 0); + BEAST_EXPECT(buffer_size(b.data()) == 0); } // static_buffer @@ -277,13 +139,53 @@ public: } (b.base()); } + + // This exercises the wrap-around cases + // for the circular buffer representation + { + static_buffer<5> b; + { + auto const mb = b.prepare(5); + BEAST_EXPECT(buffers_length(mb) == 1); + } + b.commit(4); + BEAST_EXPECT(buffers_length(b.data()) == 1); + BEAST_EXPECT(buffers_length(b.cdata()) == 1); + b.consume(3); + { + auto const mb = b.prepare(3); + BEAST_EXPECT(buffers_length(mb) == 2); + auto it1 = mb.begin(); + auto it2 = std::next(it1); + BEAST_EXPECT( + net::const_buffer(*it1).data() > + net::const_buffer(*it2).data()); + } + b.commit(2); + { + auto const mb = b.data(); + auto it1 = mb.begin(); + auto it2 = std::next(it1); + BEAST_EXPECT( + net::const_buffer(*it1).data() > + net::const_buffer(*it2).data()); + } + { + auto const cb = b.cdata(); + auto it1 = cb.begin(); + auto it2 = std::next(it1); + BEAST_EXPECT( + net::const_buffer(*it1).data() > + net::const_buffer(*it2).data()); + } + } } - void run() override + void + run() override { - testBuffer(); - testStaticBuffer(); - testMutableData>(); + testDynamicBuffer(); + testMembers(); } };