diff --git a/include/boost/capy/read.hpp b/include/boost/capy/read.hpp index 9df99cc18..3abcdf280 100644 --- a/include/boost/capy/read.hpp +++ b/include/boost/capy/read.hpp @@ -20,6 +20,7 @@ #include #include +#include #include namespace boost { @@ -113,6 +114,26 @@ read(S& stream, MB buffers) -> co_return {{}, total_read}; } +namespace detail { + +// Clamp a prepare request for the dynamic-buffer read loops. +// +// `available` is `max_size() - size()`. The request is clamped to it so that +// `size() + request` never exceeds `max_size()`. A degenerate `initial_amount` +// of 0 would otherwise make `prepare(0)` return an empty buffer and spin +// forever with no progress, so the result is floored to 1. +inline +std::size_t +read_prepare_amount( + std::size_t amount, + std::size_t available) noexcept +{ + std::size_t const n = (std::min)(amount, available); + return n != 0 ? n : 1; +} + +} // namespace detail + /** Read all data from a stream into a dynamic buffer. @par Await-effects @@ -145,12 +166,19 @@ read(S& stream, MB buffers) -> @par Await-throws - - Whatever operations on @c dunbuf throw. - (Note: types modeling @c DynamicBufferParam provided by Capy throw - @c std::bad_alloc from member function - @c prepare .) + Whatever operations on @c dynbuf throw. + + This algorithm relies on @c dynbuf.prepare(n) accepting any @c n for which + `dynbuf.size() + n <= dynbuf.max_size()`. The growable buffers + (@ref string_dynamic_buffer , @ref vector_dynamic_buffer ) honor this by + reallocating, and @ref circular_dynamic_buffer by wrapping. A fixed-capacity + buffer is permitted, but not required, to compact; one that does not can + have `capacity() < max_size() - size()` after a partial @c consume (e.g. a + reused @ref flat_dynamic_buffer ). Preparing into that gap throws + (@c std::invalid_argument or @c std::length_error ), so such a buffer must be + passed without a previously consumed prefix. Allocation failure in a growable + buffer surfaces as @c std::bad_alloc . @param stream The stream to read from. If the lifetime of `stream` ends @@ -159,8 +187,12 @@ read(S& stream, MB buffers) -> @param dynbuf The dynamic buffer to append data to. If the lifetime of the buffer sequence represented by `dynbuf` ends before the coroutine finishes, the behavior is undefined. - @param initial_amount Hint for the value to be passed in the initial call to `dynbuf.prepare()` - (default 2048). + @param initial_amount Hint for the value passed to `dynbuf.prepare()` + (default 2048; a value of 0 is treated as 1). There is no precondition + relating `initial_amount` to `dynbuf.max_size()`: the requested amount is + clamped so that `dynbuf.size()` plus the request never exceeds + `dynbuf.max_size()`. Reaching `max_size()` completes the operation + successfully. @par Remarks @@ -194,7 +226,12 @@ read( std::size_t total_read = 0; for(;;) { - auto mb = dynbuf.prepare(amount); + if(dynbuf.size() >= dynbuf.max_size()) + co_return {{}, total_read}; + + std::size_t const available = dynbuf.max_size() - dynbuf.size(); + auto mb = dynbuf.prepare( + detail::read_prepare_amount(amount, available)); auto const mb_size = buffer_size(mb); auto [ec, n] = co_await stream.read_some(mb); dynbuf.commit(n); @@ -241,13 +278,20 @@ read( @par Await-throws - - Whatever operations on @c dunbuf throw. - (Note: types modeling @c DynamicBufferParam provided by Capy throw - @c std::bad_alloc from member function - @c prepare .) - + Whatever operations on @c dynbuf throw. + + This algorithm relies on @c dynbuf.prepare(n) accepting any @c n for which + `dynbuf.size() + n <= dynbuf.max_size()`. The growable buffers + (@ref string_dynamic_buffer , @ref vector_dynamic_buffer ) honor this by + reallocating, and @ref circular_dynamic_buffer by wrapping. A fixed-capacity + buffer is permitted, but not required, to compact; one that does not can + have `capacity() < max_size() - size()` after a partial @c consume (e.g. a + reused @ref flat_dynamic_buffer ). Preparing into that gap throws + (@c std::invalid_argument or @c std::length_error ), so such a buffer must be + passed without a previously consumed prefix. Allocation failure in a growable + buffer surfaces as @c std::bad_alloc . + @param source The source to read from. If the lifetime of `source` ends before the coroutine finishes, the behavior is undefined. @@ -256,8 +300,12 @@ read( buffer sequence represented by `dynbuf` ends before the coroutine finishes, the behavior is undefined. - @param initial_amount Hint for the value to be passed in the initial call to `dynbuf.prepare()` - (default 2048). + @param initial_amount Hint for the value passed to `dynbuf.prepare()` + (default 2048; a value of 0 is treated as 1). There is no precondition + relating `initial_amount` to `dynbuf.max_size()`: the requested amount is + clamped so that `dynbuf.size()` plus the request never exceeds + `dynbuf.max_size()`. Reaching `max_size()` completes the operation + successfully. @par Remarks Supports _IoAwaitable cancellation_. @@ -290,7 +338,12 @@ read( std::size_t total_read = 0; for(;;) { - auto mb = dynbuf.prepare(amount); + if(dynbuf.size() >= dynbuf.max_size()) + co_return {{}, total_read}; + + std::size_t const available = dynbuf.max_size() - dynbuf.size(); + auto mb = dynbuf.prepare( + detail::read_prepare_amount(amount, available)); auto const mb_size = buffer_size(mb); auto [ec, n] = co_await source.read(mb); dynbuf.commit(n); diff --git a/test/unit/read.cpp b/test/unit/read.cpp index 53f118992..944e3a741 100644 --- a/test/unit/read.cpp +++ b/test/unit/read.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -23,7 +24,9 @@ #include #include +#include #include +#include namespace boost { namespace capy { @@ -754,6 +757,143 @@ struct read_test testStreamDynBufCircular(); } + //---------------------------------------------------------- + // Bounded dynamic buffer: reaching max_size completes the + // transfer successfully and never throws (issue #318). Before + // the fix, prepare() threw std::invalid_argument (string) or + // std::length_error (circular) when the requested amount + // exceeded the remaining capacity. + //---------------------------------------------------------- + + void + testDynBufMaxSize() + { + // These cases verify the deterministic max_size behavior, so they + // use inert() (a single fault-free run) rather than armed(). + + // ReadStream, string buffer: default initial_amount (2048) + // far exceeds max_size; data exceeds max_size. Fills to + // max_size and stops; no throw. + BOOST_TEST(test::fuse().inert([](test::fuse& f) -> task + { + test::read_stream rs(f); + rs.provide("abcdef"); + + std::string s; + string_dynamic_buffer db(&s, 4); + auto [ec, n] = co_await read(rs, db); + BOOST_TEST(! ec); + BOOST_TEST_EQ(n, 4u); + BOOST_TEST_EQ(s, "abcd"); + })); + + // ReadStream, string buffer: explicit initial_amount > max_size. + BOOST_TEST(test::fuse().inert([](test::fuse& f) -> task + { + test::read_stream rs(f); + rs.provide("hello world"); + + std::string s; + string_dynamic_buffer db(&s, 4); + auto [ec, n] = co_await read(rs, db, 100); + BOOST_TEST(! ec); + BOOST_TEST_EQ(n, 4u); + BOOST_TEST_EQ(s, "hell"); + })); + + // ReadStream, string buffer: EOF before max_size is reached. + // eof remains a success (n is the bytes read so far). + BOOST_TEST(test::fuse().inert([](test::fuse& f) -> task + { + test::read_stream rs(f); + rs.provide("ab\n"); + + std::string s; + string_dynamic_buffer db(&s, 4); + auto [ec, n] = co_await read(rs, db, 1); + BOOST_TEST(! ec); + BOOST_TEST_EQ(n, 3u); + BOOST_TEST_EQ(s, "ab\n"); + })); + + // ReadStream, circular buffer: data exceeds max_size. Exercises + // the std::length_error path that used to throw. + BOOST_TEST(test::fuse().inert([](test::fuse& f) -> task + { + test::read_stream rs(f); + rs.provide("abcdef"); + + char storage[4]; + circular_dynamic_buffer db(storage, sizeof(storage)); + auto [ec, n] = co_await read(rs, db); + BOOST_TEST(! ec); + BOOST_TEST_EQ(n, 4u); + std::string out; + for(auto const& b : db.data()) + out.append( + static_cast(b.data()), b.size()); + BOOST_TEST_EQ(out, "abcd"); + })); + + // ReadStream, flat buffer (fresh, no consumed prefix): fills to + // max_size and stops; no throw. + BOOST_TEST(test::fuse().inert([](test::fuse& f) -> task + { + test::read_stream rs(f); + rs.provide("abcdef"); + + char storage[4]; + flat_dynamic_buffer db(storage, sizeof(storage)); + auto [ec, n] = co_await read(rs, db); + BOOST_TEST(! ec); + BOOST_TEST_EQ(n, 4u); + BOOST_TEST_EQ(std::string_view(storage, 4), "abcd"); + })); + + // ReadStream, flat buffer reused after a partial consume: it does not + // compact, so capacity() (0) < max_size()-size() (4) and prepare + // throws. This documents the no-compaction limitation (issue #318): + // such a buffer must be passed without a previously consumed prefix. + BOOST_TEST(test::fuse().inert([](test::fuse& f) -> task + { + test::read_stream rs(f); + rs.provide("xyz"); + + char storage[8] = {}; + // 8 bytes readable, then consume 4: in_pos_=4, size()=4, + // capacity()=0, max_size()=8. + flat_dynamic_buffer db( + storage, sizeof(storage), sizeof(storage)); + db.consume(4); + + bool threw = false; + try + { + auto r = co_await read(rs, db); + (void)r; + } + catch(std::invalid_argument const&) + { + threw = true; + } + BOOST_TEST(threw); + })); + + // ReadSource overload: same clamping applies. + BOOST_TEST(test::fuse().inert([](test::fuse& f) -> task + { + test::read_source rs(f); + rs.provide("abcdef"); + + std::string s; + string_dynamic_buffer db(&s, 4); + auto [ec, n] = co_await read(rs, db); + BOOST_TEST(! ec); + BOOST_TEST_EQ(n, 4u); + BOOST_TEST_EQ(s, "abcd"); + })); + } + //---------------------------------------------------------- void @@ -762,6 +902,7 @@ struct read_test testReadStream(); testReadSource(); testStreamDynBuf(); + testDynBufMaxSize(); } };