From 4cfe860b9330be7065aff9504ec0f177b07c0c50 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Sat, 15 Dec 2018 07:37:19 -0800 Subject: [PATCH] Refactor buffers_prefix: * Tidy up tests * Increase code coverage * Correct behavior for default constructed iterators --- CHANGELOG.md | 1 + include/boost/beast/core/buffers_prefix.hpp | 22 +- .../boost/beast/core/impl/buffers_prefix.hpp | 39 ++-- test/beast/core/buffer_test.hpp | 1 + test/beast/core/buffers_prefix.cpp | 203 +++++------------- 5 files changed, 89 insertions(+), 177 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28f1d527..576bf78e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Version 200 * Refactor buffers_adaptor * Refactor buffers_range * Fix and refactor buffers_cat +* Refactor buffers_prefix API Changes: diff --git a/include/boost/beast/core/buffers_prefix.hpp b/include/boost/beast/core/buffers_prefix.hpp index 0548005a..66017458 100644 --- a/include/boost/beast/core/buffers_prefix.hpp +++ b/include/boost/beast/core/buffers_prefix.hpp @@ -11,8 +11,8 @@ #define BOOST_BEAST_BUFFERS_PREFIX_HPP #include +#include #include -#include #include // for in_place_init_t #include #include @@ -49,13 +49,19 @@ class buffers_prefix_view std::size_t dist); public: - /// The type for each element in the list of buffers. - using value_type = typename std::conditional< - std::is_convertible::value_type, - net::mutable_buffer>::value, - net::mutable_buffer, - net::const_buffer>::type; + /** The type for each element in the list of buffers. + + If the type of the underlying sequence is a mutable buffer + sequence, then `value_type` is `net::mutable_buffer`. Otherwise, + `value_type` is `net::const_buffer`. + + @see buffers_type + */ +#if BOOST_BEAST_DOXYGEN + using value_type = __see_below__; +#else + using value_type = buffers_type; +#endif #if BOOST_BEAST_DOXYGEN /// A bidirectional iterator type that may be used to read elements. diff --git a/include/boost/beast/core/impl/buffers_prefix.hpp b/include/boost/beast/core/impl/buffers_prefix.hpp index 7db402dc..f5b99da3 100644 --- a/include/boost/beast/core/impl/buffers_prefix.hpp +++ b/include/boost/beast/core/impl/buffers_prefix.hpp @@ -26,7 +26,7 @@ class buffers_prefix_view::const_iterator friend class buffers_prefix_view; buffers_prefix_view const* b_ = nullptr; - std::size_t remain_; + std::size_t remain_ = 0; iter_type it_; public: @@ -49,20 +49,7 @@ public: bool operator==(const_iterator const& other) const { - return - (b_ == nullptr) ? - ( - other.b_ == nullptr || - other.it_ == other.b_->end_ - ):( - (other.b_ == nullptr) ? - ( - it_ == b_->end_ - ): ( - b_ == other.b_ && - it_ == other.it_ - ) - ); + return b_ == other.b_ && it_ == other.it_; } bool @@ -111,16 +98,18 @@ public: } private: - const_iterator(buffers_prefix_view const& b, - std::true_type) + const_iterator( + buffers_prefix_view const& b, + std::true_type) : b_(&b) , remain_(b.remain_) , it_(b_->end_) { } - const_iterator(buffers_prefix_view const& b, - std::false_type) + const_iterator( + buffers_prefix_view const& b, + std::false_type) : b_(&b) , remain_(b_->size_) , it_(net::buffer_sequence_begin(b_->bs_)) @@ -222,18 +211,22 @@ buffers_prefix_view( template auto -buffers_prefix_view::begin() const -> +buffers_prefix_view:: +begin() const -> const_iterator { - return const_iterator{*this, std::false_type{}}; + return const_iterator{ + *this, std::false_type{}}; } template auto -buffers_prefix_view::end() const -> +buffers_prefix_view:: +end() const -> const_iterator { - return const_iterator{*this, std::true_type{}}; + return const_iterator{ + *this, std::true_type{}}; } template diff --git a/test/beast/core/buffer_test.hpp b/test/beast/core/buffer_test.hpp index ec428ce4..592cbf94 100644 --- a/test/beast/core/buffer_test.hpp +++ b/test/beast/core/buffer_test.hpp @@ -205,6 +205,7 @@ void test_mutable_buffers( { using net::buffer_size; string_view src = "Hello, world!"; + BOOST_ASSERT(buffer_size(b) <= src.size()); if(src.size() > buffer_size(b)) src = {src.data(), buffer_size(b)}; net::buffer_copy(b, net::const_buffer( diff --git a/test/beast/core/buffers_prefix.cpp b/test/beast/core/buffers_prefix.cpp index 6971ad4d..9890ee60 100644 --- a/test/beast/core/buffers_prefix.cpp +++ b/test/beast/core/buffers_prefix.cpp @@ -10,11 +10,11 @@ // Test that header file is self-contained. #include -#include +#include "buffer_test.hpp" + #include #include #include -#include #include namespace boost { @@ -23,109 +23,19 @@ namespace beast { class buffers_prefix_test : public beast::unit_test::suite { public: - template - static - std::size_t - bsize1(ConstBufferSequence const& bs) + void + testBufferSequence() { - std::size_t n = 0; - for(auto it = bs.begin(); it != bs.end(); ++it) - n += net::buffer_size(*it); - return n; + char buf[13]; + auto const b = + buffers_triple(buf, sizeof(buf)); + for(std::size_t i = 1; i <= sizeof(buf); ++i) + test_buffer_sequence(*this, + buffers_prefix(i, b)); } - template - static - std::size_t - bsize2(ConstBufferSequence const& bs) - { - std::size_t n = 0; - for(auto it = bs.begin(); it != bs.end(); it++) - n += net::buffer_size(*it); - return n; - } - - template - static - std::size_t - bsize3(ConstBufferSequence const& bs) - { - std::size_t n = 0; - for(auto it = bs.end(); it != bs.begin();) - n += net::buffer_size(*--it); - return n; - } - - template - static - std::size_t - bsize4(ConstBufferSequence const& bs) - { - std::size_t n = 0; - for(auto it = bs.end(); it != bs.begin();) - { - it--; - n += net::buffer_size(*it); - } - return n; - } - - template - void testMatrix() - { - using net::buffer_size; - std::string s = "Hello, world"; - BEAST_EXPECT(s.size() == 12); - for(std::size_t x = 1; x < 4; ++x) { - for(std::size_t y = 1; y < 4; ++y) { - std::size_t z = s.size() - (x + y); - { - std::array bs{{ - BufferType{&s[0], x}, - BufferType{&s[x], y}, - BufferType{&s[x+y], z}}}; - for(std::size_t i = 0; i <= s.size() + 1; ++i) - { - auto pb = buffers_prefix(i, bs); - BEAST_EXPECT(buffers_to_string(pb) == s.substr(0, i)); - auto pb2 = pb; - BEAST_EXPECT(buffers_to_string(pb2) == buffers_to_string(pb)); - pb = buffers_prefix(0, bs); - pb2 = pb; - BEAST_EXPECT(buffer_size(pb2) == 0); - pb2 = buffers_prefix(i, bs); - BEAST_EXPECT(buffers_to_string(pb2) == s.substr(0, i)); - } - } - }} - } - - void testEmptyBuffers() - { - using net::buffer_size; - - auto pb0 = buffers_prefix(0, net::mutable_buffer{}); - BEAST_EXPECT(buffer_size(pb0) == 0); - auto pb1 = buffers_prefix(1, net::mutable_buffer{}); - BEAST_EXPECT(buffer_size(pb1) == 0); - BEAST_EXPECT(net::buffer_copy(pb0, pb1) == 0); - -#if 0 - using pb_type = decltype(pb0); - buffers_suffix cb(pb0); - BEAST_EXPECT(buffer_size(cb) == 0); - BEAST_EXPECT(net::buffer_copy(cb, pb1) == 0); - cb.consume(1); - BEAST_EXPECT(buffer_size(cb) == 0); - BEAST_EXPECT(net::buffer_copy(cb, pb1) == 0); - - auto pbc = buffers_prefix(2, cb); - BEAST_EXPECT(buffer_size(pbc) == 0); - BEAST_EXPECT(net::buffer_copy(pbc, cb) == 0); -#endif - } - - void testInPlaceInit() + void + testInPlaceInit() { { class test_buffers @@ -166,6 +76,7 @@ public: 2, boost::in_place_init, c, sizeof(c)); BEAST_EXPECT(buffer_size(v) == 2); } + { char c[2]; buffers_prefix_view v( @@ -174,56 +85,56 @@ public: } } - void testIterators() + template + void + testPrefixes() { - char b[3]; - std::array bs{{ - net::const_buffer{&b[0], 1}, - net::const_buffer{&b[1], 1}, - net::const_buffer{&b[2], 1}}}; - auto pb = buffers_prefix(2, bs); - BEAST_EXPECT(bsize1(pb) == 2); - BEAST_EXPECT(bsize2(pb) == 2); - BEAST_EXPECT(bsize3(pb) == 2); - BEAST_EXPECT(bsize4(pb) == 2); + using net::buffer_size; + std::string s = "Hello, world"; + BEAST_EXPECT(s.size() == 12); + for(std::size_t x = 1; x < 4; ++x) { + for(std::size_t y = 1; y < 4; ++y) { + { + std::size_t z = s.size() - (x + y); + std::array bs{{ + BufferType{&s[0], x}, + BufferType{&s[x], y}, + BufferType{&s[x+y], z}}}; + for(std::size_t i = 0; i <= s.size() + 1; ++i) + { + auto pb = buffers_prefix(i, bs); + BEAST_EXPECT(buffers_to_string(pb) == s.substr(0, i)); + auto pb2 = pb; + BEAST_EXPECT(buffers_to_string(pb2) == buffers_to_string(pb)); + pb = buffers_prefix(0, bs); + pb2 = pb; + BEAST_EXPECT(buffer_size(pb2) == 0); + pb2 = buffers_prefix(i, bs); + BEAST_EXPECT(buffers_to_string(pb2) == s.substr(0, i)); + } + } + } } + } + + void testEmpty() + { + using net::buffer_size; + + auto pb0 = buffers_prefix(0, net::mutable_buffer{}); + BEAST_EXPECT(buffer_size(pb0) == 0); + auto pb1 = buffers_prefix(1, net::mutable_buffer{}); + BEAST_EXPECT(buffer_size(pb1) == 0); + BEAST_EXPECT(net::buffer_copy(pb0, pb1) == 0); } void - testDefaultIterators() + run() override { - // default ctor is one past the end - char b[3]; - std::array bs{{ - net::const_buffer{&b[0], 1}, - net::const_buffer{&b[1], 1}, - net::const_buffer{&b[2], 1}}}; - auto pb = buffers_prefix(2, bs); - decltype(pb)::const_iterator it; - BEAST_EXPECT(pb.end() == it); - BEAST_EXPECT(it == pb.end()); - decltype(pb)::const_iterator it2; - BEAST_EXPECT(it == it2); - BEAST_EXPECT(it2 == it); - it = pb.end(); - it2 = pb.end(); - BEAST_EXPECT(it == it2); - BEAST_EXPECT(it2 == it); - decltype(pb)::const_iterator it3(it2); - BEAST_EXPECT(it3 == it2); - it = pb.begin(); - BEAST_EXPECT(it != it3); - it = it3; - BEAST_EXPECT(it == it3); - } - - void run() override - { - testMatrix(); - testMatrix(); - testEmptyBuffers(); + testBufferSequence(); testInPlaceInit(); - testIterators(); - testDefaultIterators(); + testPrefixes(); + testPrefixes(); + testEmpty(); } };