Refactor ostream:

* Fix overflow
* Better calculation for prepare, no read_size
* Improve tests and code coverage

CHANGELOG.md

@@ -17,6 +17,7 @@ Version 200
 * Tidy up flat_static_buffer tests
 * Add more tests for dynamic buffers
 * Tidy up multi_buffer
+* Refactor ostream
 
 API Changes:
 

include/boost/beast/core/detail/ostream.hpp

@@ -12,8 +12,8 @@
 
 #include <boost/beast/core/buffers_prefix.hpp>
 #include <boost/beast/core/buffers_range.hpp>
-#include <boost/beast/core/read_size.hpp>
 #include <boost/beast/core/detail/type_traits.hpp>
+#include <boost/throw_exception.hpp>
 #include <boost/asio/buffer.hpp>
 #include <memory>
 #include <iosfwd>
@@ -50,7 +50,8 @@ operator<<(std::ostream& os,
     buffers_helper<Buffers> const& v)
 {
     for(auto b : buffers_range(std::ref(v.b_)))
-        os.write(static_cast<char const*>(b.data()), b.size());
+        os.write(static_cast<char const*>(
+            b.data()), b.size());
     return os;
 }
 
@@ -98,55 +99,40 @@ public:
     ostream_buffer(DynamicBuffer& b)
         : b_(b)
     {
-        prepare();
     }
 
     int_type
     overflow(int_type ch) override
     {
-        if(! Traits::eq_int_type(ch, Traits::eof()))
-        {
-            Traits::assign(*this->pptr(),
-                static_cast<CharT>(ch));
-            flush(1);
-            prepare();
-            return ch;
-        }
-        flush();
-        return traits_type::eof();
-    }
+        BOOST_ASSERT(! Traits::eq_int_type(
+            ch, Traits::eof()));
+        sync();
 
-    int
-    sync() override
-    {
-        flush();
-        prepare();
-        return 0;
-    }
-
-private:
-    void
-    prepare()
-    {
         static std::size_t constexpr max_size = 65536;
         auto const max_prepare = std::min<std::size_t>(
             std::max<std::size_t>(
                 512, b_.capacity() - b_.size()),
             std::min<std::size_t>(
                 max_size, b_.max_size() - b_.size()));
+        if(max_prepare == 0)
+            return Traits::eof();
         auto const bs = b_.prepare(max_prepare);
         auto const b = buffers_front(bs);
         auto const p = static_cast<CharT*>(b.data());
-        this->setp(p,
-            p + b.size() / sizeof(CharT) - 1);
+        this->setp(p, p + b.size() / sizeof(CharT));
+
+        BOOST_ASSERT(b_.capacity() > b_.size());
+        return this->sputc(
+            Traits::to_char_type(ch));
     }
 
-    void
-    flush(int extra = 0)
+    int
+    sync() override
     {
         b_.commit(
-            (this->pptr() - this->pbase() + extra) *
-                sizeof(CharT));
+            (this->pptr() - this->pbase()) *
+            sizeof(CharT));
+        return 0;
     }
 };
 
@@ -180,55 +166,40 @@ public:
     ostream_buffer(DynamicBuffer& b)
         : b_(b)
     {
-        prepare();
     }
 
     int_type
     overflow(int_type ch) override
     {
-        if(! Traits::eq_int_type(ch, Traits::eof()))
-        {
-            Traits::assign(*this->pptr(),
-                static_cast<CharT>(ch));
-            flush(1);
-            prepare();
-            return ch;
-        }
-        flush();
-        return traits_type::eof();
-    }
+        BOOST_ASSERT(! Traits::eq_int_type(
+            ch, Traits::eof()));
+        sync();
 
-    int
-    sync() override
-    {
-        flush();
-        prepare();
-        return 0;
-    }
-
-private:
-    void
-    prepare()
-    {
         static std::size_t constexpr max_size = 65536;
         auto const max_prepare = std::min<std::size_t>(
             std::max<std::size_t>(
                 512, b_.capacity() - b_.size()),
             std::min<std::size_t>(
                 max_size, b_.max_size() - b_.size()));
+        if(max_prepare == 0)
+            return Traits::eof();
         auto const bs = b_.prepare(max_prepare);
         auto const b = buffers_front(bs);
         auto const p = static_cast<CharT*>(b.data());
-        this->setp(p,
-            p + b.size() / sizeof(CharT) - 1);
+        this->setp(p, p + b.size() / sizeof(CharT));
+
+        BOOST_ASSERT(b_.capacity() > b_.size());
+        return this->sputc(
+            Traits::to_char_type(ch));
     }
 
-    void
-    flush(int extra = 0)
+    int
+    sync() override
     {
         b_.commit(
-            (this->pptr() - this->pbase() + extra) *
-                sizeof(CharT));
+            (this->pptr() - this->pbase()) *
+            sizeof(CharT));
+        return 0;
     }
 };
 
@@ -256,16 +227,14 @@ public:
 template<class DynamicBuffer, class CharT, class Traits>
 ostream_helper<DynamicBuffer, CharT, Traits, true>::
 ostream_helper(DynamicBuffer& b)
-    : std::basic_ostream<CharT, Traits>(
-        &this->osb_)
+    : std::basic_ostream<CharT, Traits>(&this->osb_)
     , osb_(b)
 {
 }
 
 template<class DynamicBuffer, class CharT, class Traits>
 ostream_helper<DynamicBuffer, CharT, Traits, true>::
-ostream_helper(
-        ostream_helper&& other)
+ostream_helper(ostream_helper&& other)
     : std::basic_ostream<CharT, Traits>(&osb_)
     , osb_(std::move(other.osb_))
 {
@@ -304,7 +273,8 @@ public:
             DynamicBuffer, CharT, Traits, false>>(
                 new ostream_buffer<DynamicBuffer,
                     CharT, Traits, false>(b))
-        , std::basic_ostream<CharT, Traits>(this->member.get())
+        , std::basic_ostream<CharT, Traits>(
+            this->member.get())
     {
     }
 
@@ -312,7 +282,8 @@ public:
         : ostream_helper_base<ostream_buffer<
             DynamicBuffer, CharT, Traits, false>>(
                 std::move(other))
-        , std::basic_ostream<CharT, Traits>(this->member.get())
+        , std::basic_ostream<CharT, Traits>(
+            this->member.get())
     {
     }
 };

test/beast/core/buffers_adaptor.cpp

@@ -14,6 +14,7 @@
 
 #include <boost/beast/core/multi_buffer.hpp>
 #include <boost/beast/core/ostream.hpp>
+#include <boost/beast/core/read_size.hpp>
 #include <boost/beast/_experimental/unit_test/suite.hpp>
 #include <boost/asio/buffer.hpp>
 #include <boost/asio/streambuf.hpp>

test/beast/core/flat_static_buffer.cpp

@@ -13,6 +13,7 @@
 #include "buffer_test.hpp"
 
 #include <boost/beast/core/ostream.hpp>
+#include <boost/beast/core/read_size.hpp>
 #include <boost/beast/core/string.hpp>
 #include <boost/beast/_experimental/unit_test/suite.hpp>
 #include <boost/asio/buffers_iterator.hpp>

test/beast/core/multi_buffer.cpp

@@ -13,6 +13,7 @@
 #include "buffer_test.hpp"
 
 #include <boost/beast/core/ostream.hpp>
+#include <boost/beast/core/read_size.hpp>
 #include <boost/beast/core/string.hpp>
 #include <boost/beast/core/type_traits.hpp>
 #include <boost/beast/test/test_allocator.hpp>

test/beast/core/ostream.cpp

@@ -11,6 +11,8 @@
 #include <boost/beast/core/ostream.hpp>
 
 #include <boost/beast/core/buffers_to_string.hpp>
+#include <boost/beast/core/flat_buffer.hpp>
+#include <boost/beast/core/flat_static_buffer.hpp>
 #include <boost/beast/core/multi_buffer.hpp>
 #include <boost/beast/_experimental/unit_test/suite.hpp>
 #include <ostream>
@@ -22,30 +24,53 @@ class ostream_test : public beast::unit_test::suite
 {
 public:
     void
-    run() override
+    testOstream()
     {
+        string_view const s = "0123456789abcdef";
+        BEAST_EXPECT(s.size() == 16);
+
+        // overflow
         {
-            multi_buffer b;
-            auto os = ostream(b);
-            os << "Hello, world!\n";
-            os.flush();
-            BEAST_EXPECT(buffers_to_string(b.data()) == "Hello, world!\n");
-            auto os2 = std::move(os);
-        }
-        {
-            auto const s =
-                "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" 
-                "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" 
-                "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" 
-                "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" 
-                "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" 
-                "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" 
-                "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" 
-                "0123456789abcdef" "0123456789abcdef" "0123456789abcdef" "0123456789abcdef";
-            multi_buffer b;
+            flat_static_buffer<16> b;
             ostream(b) << s;
             BEAST_EXPECT(buffers_to_string(b.data()) == s);
         }
+
+        // max_size
+        {
+            flat_static_buffer<16> b;
+            auto os = ostream(b);
+            os << s;
+            os << '*';
+            BEAST_EXPECT(os.bad());
+        }
+
+        // max_size (exception
+        {
+            flat_static_buffer<16> b;
+            auto os = ostream(b);
+            os.exceptions(os.badbit);
+            os << s;
+            try
+            {
+                os << '*';
+                fail("missing exception", __FILE__, __LINE__);
+            }
+            catch(std::ios_base::failure const&)
+            {
+                pass();
+            }
+            catch(...)
+            {
+                fail("wrong exception", __FILE__, __LINE__);
+            }
+        }
+    }
+
+    void
+    run() override
+    {
+        testOstream();
     }
 };
 

test/beast/core/static_buffer.cpp

@@ -13,6 +13,7 @@
 #include "buffer_test.hpp"
 
 #include <boost/beast/core/ostream.hpp>
+#include <boost/beast/core/read_size.hpp>
 #include <boost/beast/core/string.hpp>
 #include <boost/beast/_experimental/unit_test/suite.hpp>
 #include <boost/asio/buffers_iterator.hpp>