Add read overload for ReadStream + DynamicBuffer #114
Conversation
Add a new read() function that reads from a ReadStream into a DynamicBufferParam until EOF. The buffer grows using a 1.5x strategy starting from an optional initial_amount (default 2048 bytes). Add corresponding unit tests covering: - Reading all data until EOF - Large data reads (growth strategy) - Empty streams - Custom initial_amount parameter - Chunked reads with max_read_size
📝 WalkthroughWalkthroughA new overload of the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Task as Task/Async
participant Stream as ReadStream
participant Buffer as DynamicBuffer
participant Error as Error Handler
Caller->>Task: read(stream, buffer)
activate Task
loop Until EOF or Error
Task->>Buffer: prepare(growth_size)
activate Buffer
Buffer-->>Task: buffer region
deactivate Buffer
Task->>Stream: read from stream
activate Stream
Stream-->>Task: bytes read / error
deactivate Stream
alt Success
Task->>Buffer: commit(bytes_read)
activate Buffer
Buffer-->>Task: confirmed
deactivate Buffer
Task->>Task: update total_bytes
else EOF or Error
Task->>Error: handle result
activate Error
Error-->>Task: io_result
deactivate Error
end
end
Task-->>Caller: task<io_result<size_t>>
deactivate Task
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
An automated preview of the documentation is available at https://114.capy.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-01-28 22:38:35 UTC |
![coderabbitai[bot]](https://d.iamgay.cn/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
include/boost/capy/read.hpp (1)
13-25: Add a high-level implementation overview block after includes.This header contains non-trivial read loops and buffer-growth logic, but there is no
/* */overview comment after the includes as required.💡 Suggested overview block
`#include` <system_error> `#include` <cstddef> +/* + * Overview: + * - Provides read() overloads for buffer sequences and dynamic buffers. + * - Dynamic-buffer overloads use prepare/commit and grow by 1.5x on full reads. + * - EOF returns success with total bytes read; other errors propagate with partial count. + */ namespace boost { namespace capy {
🤖 Fix all issues with AI agents
In `@include/boost/capy/read.hpp`:
- Around line 90-120: Update the read function documentation to use the standard
io_result format for the return value and add explicit descriptions for the
parameters and preconditions: document that `read(ReadStream auto& stream,
DynamicBufferParam buffers, std::size_t initial_amount)` reads until EOF or
error and returns io_result<std::size_t> where the io_result's .error() holds
the std::error_code and .value() holds the total bytes read; state that
`initial_amount` is the initial prepare size and explain how the buffer grows
(start at initial_amount, grow by 1.5x); add preconditions (e.g., stream must
satisfy ReadStream, buffers must meet DynamicBufferParam requirements and be
valid), note exception behavior (explicitly say that `buffers.prepare()` or
other buffer operations may throw and that such exceptions propagate), and state
thread-safety guarantees (e.g., not thread-safe unless caller-supplied
stream/buffer are concurrently safe). Reference the function name `read` and
parameters `stream`, `buffers`, and `initial_amount` in the updated doc.
- Around line 125-142: Guard against initial_amount being zero to avoid infinite
zero-length reads: in the coroutine starting at function returning
task<io_result<std::size_t>> that takes std::size_t initial_amount, check if
initial_amount == 0 and set it to a sensible minimum (e.g., 1 or a defined
MIN_READ_SIZE) or return an error before the loop so buffers.prepare(amount)
never gets called with 0; apply the same guard in the corresponding ReadSource
overload to keep behavior consistent. Ensure you reference and update the
variables amount/initial_amount and the interaction with buffers.prepare(...)
and stream.read_some(...) so the loop always makes progress.
- Around line 122-125: The coroutine read(...) currently takes the
DynamicBufferParam as an rvalue reference (DynamicBufferParam auto&&) which can
bind to a temporary that is destroyed while the coroutine is suspended; change
the parameter to be taken by value (DynamicBufferParam auto buffers) so the
coroutine frame owns the buffer and it's safe to co_await and later call
buffers.commit(n); apply the identical change to the ReadSource overload so both
functions consistently take the buffer parameter by value.
| /** Read data from a stream into a dynamic buffer. | ||
|
|
||
| This function reads data from the stream into the dynamic buffer | ||
| until end-of-file is reached or an error occurs. Data is appended | ||
| to the buffer using prepare/commit semantics. | ||
|
|
||
| The buffer grows using a strategy that starts with `initial_amount` | ||
| bytes and grows by a factor of 1.5 when filled. | ||
|
|
||
| @param stream The stream to read from, must satisfy @ref ReadStream. | ||
| @param buffers The dynamic buffer to read into. | ||
| @param initial_amount The initial number of bytes to prepare. | ||
|
|
||
| @return A task that yields `(std::error_code, std::size_t)`. | ||
| On success (EOF reached), `ec` is default-constructed and `n` | ||
| is the total number of bytes read. On error, `ec` contains the | ||
| error code and `n` is the total number of bytes read before | ||
| the error. | ||
|
|
||
| @par Example | ||
| @code | ||
| task<void> example(ReadStream auto& stream) | ||
| { | ||
| std::string body; | ||
| auto [ec, n] = co_await read(stream, string_dynamic_buffer(&body)); | ||
| // body contains n bytes of data | ||
| } | ||
| @endcode | ||
|
|
||
| @see ReadStream, DynamicBufferParam | ||
| */ |
There was a problem hiding this comment.
Docstring needs io_result format, parameter meaning, and preconditions/exceptions/thread-safety.
The return doc doesn’t follow the required io_result format, and the description lacks preconditions, exception behavior (e.g., prepare throwing), thread-safety, and explicit io_result parameter meaning.
📝 Doc fix sketch
- `@param` buffers The dynamic buffer to read into.
- `@param` initial_amount The initial number of bytes to prepare.
+ `@param` buffers The dynamic buffer to read into (must satisfy `@ref` DynamicBufferParam).
+ `@param` initial_amount The initial number of bytes to prepare.
+ `@pre` initial_amount > 0.
+ `@throws` std::invalid_argument if `@ref` DynamicBufferParam::prepare exceeds max_size().
+ `@par` Thread Safety
+ This function is not thread-safe; ensure exclusive access to `stream` and `buffers`.
- `@return` A task that yields `(std::error_code, std::size_t)`.
+ `@return` An awaitable yielding `(error_code,std::size_t)`.
On success (EOF reached), `ec` is default-constructed and `n`
is the total number of bytes read. On error, `ec` contains the
error code and `n` is the total number of bytes read before
the error.
+ The io_result parameter T1 (std::size_t) is the total number of bytes read.🤖 Prompt for AI Agents
In `@include/boost/capy/read.hpp` around lines 90 - 120, Update the read function
documentation to use the standard io_result format for the return value and add
explicit descriptions for the parameters and preconditions: document that
`read(ReadStream auto& stream, DynamicBufferParam buffers, std::size_t
initial_amount)` reads until EOF or error and returns io_result<std::size_t>
where the io_result's .error() holds the std::error_code and .value() holds the
total bytes read; state that `initial_amount` is the initial prepare size and
explain how the buffer grows (start at initial_amount, grow by 1.5x); add
preconditions (e.g., stream must satisfy ReadStream, buffers must meet
DynamicBufferParam requirements and be valid), note exception behavior
(explicitly say that `buffers.prepare()` or other buffer operations may throw
and that such exceptions propagate), and state thread-safety guarantees (e.g.,
not thread-safe unless caller-supplied stream/buffer are concurrently safe).
Reference the function name `read` and parameters `stream`, `buffers`, and
`initial_amount` in the updated doc.
| read( | ||
| ReadStream auto& stream, | ||
| DynamicBufferParam auto&& buffers, | ||
| std::size_t initial_amount = 2048) -> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
C++ coroutines temporary lifetime binding rvalue reference parameter suspension point
💡 Result:
In a coroutine, an argument passed to a reference parameter (including an rvalue-reference T&& parameter) does not get its lifetime extended to the coroutine’s lifetime.
Key rules that combine badly
-
Temporary-to-reference-parameter lifetime rule
A temporary bound to a reference parameter in a function call “exists until the end of the full-expression containing that call” (e.g. until the end of the statementcoro(T{}) ;). It is not extended further. [1] -
Coroutine parameter storage rule
When a coroutine is created, the compiler stores parameters in the coroutine state; by-reference parameters remain references (it does not copy the referred object). So the coroutine frame can hold a reference that later dangles. [2][3]
What goes wrong at a suspension point
If the coroutine suspends (at initial_suspend or any later co_await/co_yield) and later resumes after the call expression has finished, then any T&& (or T const&, etc.) parameter that was bound to a temporary from the call is now a dangling reference → UB when used.
task f(std::string&& s) {
co_await something(); // suspension point
use(s); // UB if caller did: f(std::string{"tmp"});
}Why: std::string{"tmp"} dies at end of the full-expression containing the call, but s is just a reference stored in the coroutine frame. [1][2][3]
Safe patterns
- Take by value (then the coroutine frame owns a copy/move):
task f(std::string s); - Or require the caller to pass an object that outlives all resumes (e.g.
std::string&,shared_ptr, etc.), but don’t accept temporaries unless you copy/move them into owned storage.
Bottom line: binding a temporary to a coroutine’s T&& parameter does not “follow” the coroutine across suspension; the temporary typically dies at the end of the call statement, so any later use after a suspension is invalid. [1][2][3]
Sources:
[1] cppreference – reference initialization: temporary bound to a reference parameter lasts only to end of full-expression containing the call: (cppreference.com)
[2] Lewis Baker – coroutine parameters: by-reference parameters remain references in the coroutine frame: (lewissbaker.github.io)
[3] cppreference – coroutines: parameters copied into coroutine state; by-reference params remain references (may dangle): (cppreference.net)
Citations:
- 1: https://www.cppreference.com/w/cpp/language/reference_initialization.html?utm_source=openai
- 2: https://lewissbaker.github.io/2018/09/05/understanding-the-promise-type?utm_source=openai
- 3: https://cppreference.net/cpp/language/coroutines.html?utm_source=openai
🏁 Script executed:
# First, let's look at the file structure and the specific function in question
cd /
find . -name "read.hpp" -path "*capy*" 2>/dev/null | head -5Repository: cppalliance/capy
Length of output: 107
🏁 Script executed:
# Also search for the read function definition to understand its full implementation
rg "read\(" --type cpp --type hpp -A 30 | grep -A 30 "DynamicBufferParam auto&&" | head -100Repository: cppalliance/capy
Length of output: 87
🏁 Script executed:
# Read the file with correct extensions
cat -n ./home/jailuser/git/include/boost/capy/read.hpp | head -200Repository: cppalliance/capy
Length of output: 139
🏁 Script executed:
# Also check for DynamicBufferParam usage and co_await patterns
rg "co_await|co_yield" ./home/jailuser/git/include/boost/capy/read.hpp -B 2 -A 3Repository: cppalliance/capy
Length of output: 147
🏁 Script executed:
# Look for the ReadSource overload mentioned
rg "ReadSource" ./home/jailuser/git/include/boost/capy/read.hpp -B 3 -A 10Repository: cppalliance/capy
Length of output: 147
🏁 Script executed:
# List files to find the correct path
git ls-files | grep read.hppRepository: cppalliance/capy
Length of output: 87
🏁 Script executed:
# Or search more broadly
fd "read.hpp"Repository: cppalliance/capy
Length of output: 87
🏁 Script executed:
# Get file size first
wc -l include/boost/capy/read.hppRepository: cppalliance/capy
Length of output: 91
🏁 Script executed:
# Read the file around lines 122-125
sed -n '100,180p' include/boost/capy/read.hppRepository: cppalliance/capy
Length of output: 2594
🏁 Script executed:
# Also look for all read function overloads
rg "^\s*read\(" include/boost/capy/read.hpp -B 5 -A 20Repository: cppalliance/capy
Length of output: 2036
Take DynamicBufferParam by value instead of rvalue reference.
With DynamicBufferParam auto&&, a temporary passed by caller dies at the end of the call statement. However, the function suspends at co_await stream.read_some(mb) and then accesses buffers.commit(n) after resumption, creating undefined behavior. Taking by value ensures the coroutine frame owns the buffer:
Fix
read(
ReadStream auto& stream,
- DynamicBufferParam auto&& buffers,
+ DynamicBufferParam auto buffers,
std::size_t initial_amount = 2048) ->
task<io_result<std::size_t>>Apply the same fix to the ReadSource overload for consistency.
🤖 Prompt for AI Agents
In `@include/boost/capy/read.hpp` around lines 122 - 125, The coroutine read(...)
currently takes the DynamicBufferParam as an rvalue reference
(DynamicBufferParam auto&&) which can bind to a temporary that is destroyed
while the coroutine is suspended; change the parameter to be taken by value
(DynamicBufferParam auto buffers) so the coroutine frame owns the buffer and
it's safe to co_await and later call buffers.commit(n); apply the identical
change to the ReadSource overload so both functions consistently take the buffer
parameter by value.
| std::size_t initial_amount = 2048) -> | ||
| task<io_result<std::size_t>> | ||
| { | ||
| std::size_t amount = initial_amount; | ||
| std::size_t total_read = 0; | ||
| for(;;) | ||
| { | ||
| auto mb = buffers.prepare(amount); | ||
| auto const mb_size = buffer_size(mb); | ||
| auto [ec, n] = co_await stream.read_some(mb); | ||
| buffers.commit(n); | ||
| total_read += n; | ||
| if(ec == cond::eof) | ||
| co_return {{}, total_read}; | ||
| if(ec) | ||
| co_return {ec, total_read}; | ||
| if(n == mb_size) | ||
| amount = amount / 2 + amount; |
There was a problem hiding this comment.
Guard initial_amount == 0 to avoid a zero-length read loop.
If initial_amount is 0, prepare(0) can yield empty buffers and read_some may make no progress, leading to an infinite loop.
🛡️ Proposed guard
-read(
- ReadStream auto& stream,
- DynamicBufferParam auto&& buffers,
- std::size_t initial_amount = 2048) ->
+read(
+ ReadStream auto& stream,
+ DynamicBufferParam auto&& buffers,
+ std::size_t initial_amount = 2048) ->
task<io_result<std::size_t>>
{
+ if(initial_amount == 0)
+ co_return {std::make_error_code(std::errc::invalid_argument), 0};
std::size_t amount = initial_amount;
std::size_t total_read = 0;Consider mirroring the same guard in the ReadSource overload for consistent behavior.
🤖 Prompt for AI Agents
In `@include/boost/capy/read.hpp` around lines 125 - 142, Guard against
initial_amount being zero to avoid infinite zero-length reads: in the coroutine
starting at function returning task<io_result<std::size_t>> that takes
std::size_t initial_amount, check if initial_amount == 0 and set it to a
sensible minimum (e.g., 1 or a defined MIN_READ_SIZE) or return an error before
the loop so buffers.prepare(amount) never gets called with 0; apply the same
guard in the corresponding ReadSource overload to keep behavior consistent.
Ensure you reference and update the variables amount/initial_amount and the
interaction with buffers.prepare(...) and stream.read_some(...) so the loop
always makes progress.
2 participants
Add a new read() function that reads from a ReadStream into a DynamicBufferParam until EOF. The buffer grows using a 1.5x strategy starting from an optional initial_amount (default 2048 bytes).
Add corresponding unit tests covering:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.