Conversation
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughThis PR adds multiple example projects (async-mutex, custom-executor, parallel-tasks, strand-serialization), refactors the corosio echo-server to use corosio::tcp_socket/io_context, and reorganizes example/CMakeLists.txt with new subdirectory ordering and conditional asio inclusion. (50 words) Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
An automated preview of the documentation is available at https://195.capy.prtest.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-03-03 18:56:47 UTC |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
example/echo-server-corosio/echo_server.cpp (1)
10-15: Use a post-include/* ... */implementation overview block.This file has non-trivial coroutine/network flow, so prefer a high-level
/* */overview immediately after includes instead of banner-style//header text.As per coding guidelines: "Files containing non-trivial implementation logic should include a
/* */block comment after the includes that provides a high-level overview of how the implementation works."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example/echo-server-corosio/echo_server.cpp` around lines 10 - 15, Replace the leading banner-style '//' header in echo_server.cpp with a post-include block comment that gives a high-level implementation overview of the coroutine and network flow; remove or convert the current top-of-file // comments and instead insert a /* ... */ block immediately after the `#include` statements that explains key components (e.g., how Corosio-driven coroutines accept connections, spawn handler coroutines, echo data back, and any important lifecycle/error handling) so readers get a concise architecture summary before diving into functions like the accept loop and per-connection handler.example/custom-executor/custom_executor.cpp (1)
1-27: Consolidate banner comments into one concise post-include overview block.The decorative header blocks are verbose for an example file. Prefer a short
/* ... */implementation overview placed immediately after includes.As per coding guidelines "Files containing non-trivial implementation logic should include a
/* */block comment after the includes..." and "Keep the ascii-art to a minimum, only add dividers if the class declaration is very long."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example/custom-executor/custom_executor.cpp` around lines 1 - 27, Keep the license/copyright header at the top but remove the multi-line decorative banner comments; replace the verbose ASCII headers and scattered commentary with one concise C-style block comment placed immediately after the includes that succinctly describes the file purpose (e.g., "Custom single-threaded Executor example demonstrating the Executor concept and run loop integration with Capy"). Ensure the new comment references the key intent shown in the original text (single-threaded execution context / GUI-like run loop) and remove duplicate explanatory lines elsewhere (such as the scattered "Custom Executor Example" and "A minimal single-threaded execution context" comments) so only the single post-include overview block remains before the implementation (after the includes and before using namespace / class/function definitions).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@example/async-mutex/async_mutex.cpp`:
- Around line 34-40: The on_error lambda only catches std::exception, so
non-standard exceptions escape and can prevent done.count_down() from executing;
update on_error (the lambda capturing std::exception_ptr ep and calling
done.count_down()) to also catch all other exceptions (catch(...)) after the
current catch(std::exception const& e) and log a generic message (e.g., "Unknown
exception") so that any exception type is handled and done.count_down() always
runs regardless of the exception type.
In `@example/custom-executor/custom_executor.cpp`:
- Around line 30-31: The queue_ is accessed concurrently (run(), enqueue(),
dispatch(), post()) causing data races and executor_type's public default ctor
leaves loop_ null; fix by adding a std::mutex (e.g. queue_mutex_) and guard all
accesses to queue_ (lock in enqueue(), lock when popping in run()), ensure
dispatch()/post() call the thread-safe enqueue() only, and update
enqueue()/run() to minimize lock hold time; also remove or delete the public
default constructor of executor_type (or initialize loop_ to a valid pointer)
and add a runtime check/assert at the start of public methods (enqueue(),
dispatch(), post(), run()) to fail fast if loop_ is null so you don't
dereference it.
- Around line 86-108: The methods context(), dispatch(), and post() dereference
loop_ which can be null for a default-constructed executor_type; add explicit
precondition guards (e.g., assert(loop_ != nullptr) or a DEBUG-only check) at
the start of execution_context& context() const noexcept,
std::coroutine_handle<> dispatch(...), and void post(...) to document and
enforce the invariant before dereferencing loop_, and ensure the header includes
<cassert> (or your project’s assert utility) so calls to context(), dispatch(),
and post() fail fast with a clear message when loop_ is uninitialized.
In `@example/echo-server-corosio/echo_server.cpp`:
- Around line 79-82: The port parsing currently casts std::atoi result into
unsigned short (port) with no validation; replace this by parsing argv[1] safely
(e.g., use strtol or std::stoi with error checking), verify the parsed value is
numeric and in the valid TCP port range (1–65535) before assigning to unsigned
short port, and handle invalid input by printing an error and exiting or falling
back to the default 8080; update the code paths around the port variable and the
argc/argv handling so negative, non-numeric, or out-of-range values are rejected
rather than silently wrapped.
- Around line 24-25: The coroutine echo_session currently takes
corosio::tcp_socket& and is launched detached via run_async from accept_loop
with a stack-local peer, causing a dangling reference; change echo_session to
take corosio::tcp_socket by value (or by rvalue) so the socket is owned by the
coroutine and when you call echo_session from accept_loop pass std::move(peer)
(or otherwise move the peer into the task) to transfer ownership; update any
callsites and signatures (echo_session, accept_loop, run_async invocation)
accordingly so the coroutine no longer references a stack-local socket.
In `@example/parallel-tasks/parallel_tasks.cpp`:
- Around line 50-56: The on_error lambda can let non-std exceptions escape and
skip the completion signal; update on_error (the lambda capturing done and
taking std::exception_ptr ep) so that after rethrowing you handle both
std::exception (catch(std::exception const& e)) and a final catch(...) branch
that logs an unknown/non-std error, and ensure done.count_down() is executed
unconditionally (i.e., keep the count_down() call after the catch blocks or use
a scope guard) so completion is always signaled even for non-std exceptions.
In `@example/strand-serialization/strand_serialization.cpp`:
- Around line 34-40: The on_error lambda currently only catches std::exception
from std::rethrow_exception and will let non-std::exception payloads escape
(potentially skipping done.count_down()); update the handler (on_error) to add a
catch(...) branch after catching std::exception to handle non-standard
exceptions, log a generic error message (or rethrow info if available), and
ensure done.count_down() is executed in all paths so the latch is always
signaled even for non-std::exception errors.
---
Nitpick comments:
In `@example/custom-executor/custom_executor.cpp`:
- Around line 1-27: Keep the license/copyright header at the top but remove the
multi-line decorative banner comments; replace the verbose ASCII headers and
scattered commentary with one concise C-style block comment placed immediately
after the includes that succinctly describes the file purpose (e.g., "Custom
single-threaded Executor example demonstrating the Executor concept and run loop
integration with Capy"). Ensure the new comment references the key intent shown
in the original text (single-threaded execution context / GUI-like run loop) and
remove duplicate explanatory lines elsewhere (such as the scattered "Custom
Executor Example" and "A minimal single-threaded execution context" comments) so
only the single post-include overview block remains before the implementation
(after the includes and before using namespace / class/function definitions).
In `@example/echo-server-corosio/echo_server.cpp`:
- Around line 10-15: Replace the leading banner-style '//' header in
echo_server.cpp with a post-include block comment that gives a high-level
implementation overview of the coroutine and network flow; remove or convert the
current top-of-file // comments and instead insert a /* ... */ block immediately
after the `#include` statements that explains key components (e.g., how
Corosio-driven coroutines accept connections, spawn handler coroutines, echo
data back, and any important lifecycle/error handling) so readers get a concise
architecture summary before diving into functions like the accept loop and
per-connection handler.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
doc/modules/ROOT/nav.adocis excluded by!**/doc/**doc/modules/ROOT/pages/7.examples/7i.echo-server-corosio.adocis excluded by!**/doc/**doc/modules/ROOT/pages/7.examples/7j.stream-pipeline.adocis excluded by!**/doc/**doc/modules/ROOT/pages/7.examples/7k.strand-serialization.adocis excluded by!**/doc/**doc/modules/ROOT/pages/7.examples/7l.async-mutex.adocis excluded by!**/doc/**doc/modules/ROOT/pages/7.examples/7m.parallel-tasks.adocis excluded by!**/doc/**doc/modules/ROOT/pages/7.examples/7n.custom-executor.adocis excluded by!**/doc/**
📒 Files selected for processing (10)
example/CMakeLists.txtexample/async-mutex/CMakeLists.txtexample/async-mutex/async_mutex.cppexample/custom-executor/CMakeLists.txtexample/custom-executor/custom_executor.cppexample/echo-server-corosio/echo_server.cppexample/parallel-tasks/CMakeLists.txtexample/parallel-tasks/parallel_tasks.cppexample/strand-serialization/CMakeLists.txtexample/strand-serialization/strand_serialization.cpp
| std::queue<std::coroutine_handle<>> queue_; | ||
| std::thread::id owner_; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n example/custom-executor/custom_executor.cppRepository: cppalliance/capy
Length of output: 4568
Protect queue_ access; concurrent scheduling creates data race.
The queue_ member is accessed without synchronization. run() (lines 54–59) and enqueue() (lines 62–65, 107) mutate the queue without locks. However, dispatch() explicitly allows cross-thread enqueuing (line 99–102: enqueue is called when NOT on the owner thread), and post() unconditionally calls enqueue() from any thread. This creates a data race if post() or dispatch() is called from a thread other than the one running loop.run().
Additionally, executor_type has a public default constructor (line 86) that leaves loop_ as nullptr, causing a null-pointer dereference if any method is called on a default-constructed executor.
Suggested fix
`#include` <boost/capy.hpp>
`#include` <iostream>
+#include <mutex>
`#include` <queue>
`#include` <thread>
@@
class run_loop : public execution_context
{
std::queue<std::coroutine_handle<>> queue_;
+ mutable std::mutex queue_mtx_;
std::thread::id owner_;
@@
void run()
{
owner_ = std::this_thread::get_id();
- while (!queue_.empty())
+ while (true)
{
- auto h = queue_.front();
- queue_.pop();
+ std::coroutine_handle<> h;
+ {
+ std::lock_guard<std::mutex> lk(queue_mtx_);
+ if (queue_.empty())
+ break;
+ h = queue_.front();
+ queue_.pop();
+ }
h.resume();
}
}
@@
void enqueue(std::coroutine_handle<> h)
{
+ std::lock_guard<std::mutex> lk(queue_mtx_);
queue_.push(h);
}Also applies to: lines 51–65, 96–108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@example/custom-executor/custom_executor.cpp` around lines 30 - 31, The queue_
is accessed concurrently (run(), enqueue(), dispatch(), post()) causing data
races and executor_type's public default ctor leaves loop_ null; fix by adding a
std::mutex (e.g. queue_mutex_) and guard all accesses to queue_ (lock in
enqueue(), lock when popping in run()), ensure dispatch()/post() call the
thread-safe enqueue() only, and update enqueue()/run() to minimize lock hold
time; also remove or delete the public default constructor of executor_type (or
initialize loop_ to a valid pointer) and add a runtime check/assert at the start
of public methods (enqueue(), dispatch(), post(), run()) to fail fast if loop_
is null so you don't dereference it.
There was a problem hiding this comment.
Not addressing. This is intentionally a single-threaded run loop — the class comment and example title say so explicitly. In main(), run_async enqueues onto the loop, then loop.run() drains it, all on the same thread. Adding a mutex would add complexity that contradicts the purpose of the example (showing the minimal Executor concept implementation). The dispatch() cross-thread path exists to satisfy the concept but is not exercised here.
There was a problem hiding this comment.
@sgerbino, thank you for the clarification! You're absolutely right—the class comment and example explicitly state this is a single-threaded run loop demonstrating the minimal Executor concept. Since the example runs entirely on one thread (enqueue via run_async, then drain via loop.run()), the data race concern doesn't apply to the intended usage. I appreciate you explaining the design goal of keeping it simple for educational purposes.
✏️ Learnings added
Learnt from: sgerbino
Repo: cppalliance/capy PR: 195
File: example/custom-executor/custom_executor.cpp:30-31
Timestamp: 2026-03-03T18:46:48.319Z
Learning: The custom executor example (example/custom-executor/custom_executor.cpp) is intentionally a single-threaded run loop demonstrating minimal Executor concept implementation without thread safety mechanisms. The cross-thread path in dispatch() exists to satisfy the concept but is not exercised in the example.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: cppalliance/capy PR: 0
File: .cursor/rules/doc-concept.mdc:0-0
Timestamp: 2026-02-01T22:20:18.267Z
Learning: Applies to **/*.{hpp,h} : Concept javadoc should include par Example with a code block showing a generic algorithm templated on the concept, demonstrating typical usage with destructuring, error handling, and interaction with related concepts
Learnt from: CR
Repo: cppalliance/capy PR: 0
File: .cursor/rules/doc-general.mdc:0-0
Timestamp: 2026-02-01T22:20:39.250Z
Learning: Applies to **/*.{h,hpp,hxx} : Include par Example in function javadocs with one or more code blocks; provide more examples for functions with multiple usage patterns or overloads
| executor_type() = default; | ||
|
|
||
| execution_context& context() const noexcept | ||
| { | ||
| return *loop_; | ||
| } | ||
|
|
||
| void on_work_started() const noexcept {} | ||
| void on_work_finished() const noexcept {} | ||
|
|
||
| std::coroutine_handle<> dispatch( | ||
| std::coroutine_handle<> h) const | ||
| { | ||
| if (loop_->is_running_on_this_thread()) | ||
| return h; | ||
| loop_->enqueue(h); | ||
| return std::noop_coroutine(); | ||
| } | ||
|
|
||
| void post(std::coroutine_handle<> h) const | ||
| { | ||
| loop_->enqueue(h); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the complete executor_type class definition
cd example/custom-executor && wc -l custom_executor.cppRepository: cppalliance/capy
Length of output: 83
🏁 Script executed:
# Get the full context of the executor_type class
cd example/custom-executor && sed -n '70,120p' custom_executor.cppRepository: cppalliance/capy
Length of output: 1038
🏁 Script executed:
# Search for the executor_type class declaration and all its members
rg -n 'class executor_type|executor_type\s*\(' example/custom-executor/custom_executor.cpp -A 10 -B 2Repository: cppalliance/capy
Length of output: 690
🏁 Script executed:
# Check if loop_ is initialized or what its default value is
rg -n 'loop_\s*=|loop_\s*\(' example/custom-executor/custom_executor.cpp -B 2 -A 2Repository: cppalliance/capy
Length of output: 369
🏁 Script executed:
# Search for executor concept requirements in the codebase
rg -n 'concept.*Executor|default_initializable|semiregular' --type=cpp --type=hpp | head -30Repository: cppalliance/capy
Length of output: 87
🏁 Script executed:
# Check how executor_type is actually used (instantiated)
rg -n 'executor_type\s*\(\)|\brun_loop::executor_type\b' --type=cpp --type=hppRepository: cppalliance/capy
Length of output: 87
🏁 Script executed:
# Search for executor concept in the codebase
find . -name "*.hpp" -o -name "*.h" -o -name "*.cpp" | head -20Repository: cppalliance/capy
Length of output: 903
🏁 Script executed:
# Look for executor concept or requirements
rg -l 'concept.*Executor|executor_type' --type=cpp | head -20Repository: cppalliance/capy
Length of output: 620
🏁 Script executed:
# Search for capy executor concept
rg -n 'concept.*Executor' . -A 5 -B 2Repository: cppalliance/capy
Length of output: 4501
🏁 Script executed:
# Check tests for executor_type usage
rg -n 'executor_type\s*\(\)' . --type=cpp --type=hppRepository: cppalliance/capy
Length of output: 87
🏁 Script executed:
# Look for any documentation on executor semiregular/regular requirement
rg -n 'semiregular|regular|default.construct' . --type=cpp --type=hpp | head -20Repository: cppalliance/capy
Length of output: 87
🏁 Script executed:
# Check the include files to understand executor interface requirements
cat example/custom-executor/custom_executor.cpp | head -50Repository: cppalliance/capy
Length of output: 1216
🏁 Script executed:
# Look at the full executor concept from the capy library
cat include/boost/capy/concept/executor.hpp | grep -A 30 'concept Executor'Repository: cppalliance/capy
Length of output: 764
🏁 Script executed:
# Search for any usage of executor_type default construction
rg 'executor_type\s*\(\)|\bexecutor_type\s*\{\}' . --type=cppRepository: cppalliance/capy
Length of output: 207
🏁 Script executed:
# Check if there are any tests that default-construct executor_type
rg 'executor_type' test/ --type=cpp -B 2 -A 2Repository: cppalliance/capy
Length of output: 3299
🏁 Script executed:
# Check examples directory for executor_type usage
rg 'executor_type' example/ --type=cpp -B 2 -A 2Repository: cppalliance/capy
Length of output: 4235
🏁 Script executed:
# Get the full context of the thread_pool test that mentions default construction
sed -n '1,50p' test/unit/ex/thread_pool.cpp | cat -nRepository: cppalliance/capy
Length of output: 1689
🏁 Script executed:
# Look for the test with "valid but unassociated" comment
rg -n 'valid but unassociated|Default-constructed executor' test/unit/ex/thread_pool.cpp -B 5 -A 10Repository: cppalliance/capy
Length of output: 569
🏁 Script executed:
# Check if there's any documentation about executor null state in include/boost/capy/concept/executor.hpp
cat include/boost/capy/concept/executor.hpp | head -100Repository: cppalliance/capy
Length of output: 3681
Add precondition guards to prevent undefined behavior on default-constructed executors.
The default constructor leaves loop_ == nullptr, and context(), dispatch(), and post() dereference it unconditionally. This creates undefined behavior when these methods are called on default-constructed executors. While this appears to be intentional design (allowing default construction for standard library compatibility while accepting a "valid but unassociated state"), the example should defend against accidental misuse by explicitly guarding the dereferences with assertions that document the precondition.
Suggested defensive guards
+#include <cassert>
@@
execution_context& context() const noexcept
{
+ assert(loop_ != nullptr);
return *loop_;
}
@@
std::coroutine_handle<> dispatch(
std::coroutine_handle<> h) const
{
+ assert(loop_ != nullptr);
if (loop_->is_running_on_this_thread())
return h;
loop_->enqueue(h);
return std::noop_coroutine();
}
@@
void post(std::coroutine_handle<> h) const
{
+ assert(loop_ != nullptr);
loop_->enqueue(h);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@example/custom-executor/custom_executor.cpp` around lines 86 - 108, The
methods context(), dispatch(), and post() dereference loop_ which can be null
for a default-constructed executor_type; add explicit precondition guards (e.g.,
assert(loop_ != nullptr) or a DEBUG-only check) at the start of
execution_context& context() const noexcept, std::coroutine_handle<>
dispatch(...), and void post(...) to document and enforce the invariant before
dereferencing loop_, and ensure the header includes <cassert> (or your project’s
assert utility) so calls to context(), dispatch(), and post() fail fast with a
clear message when loop_ is uninitialized.
There was a problem hiding this comment.
Not addressing. The default constructor exists to satisfy semiregular requirements of the Executor concept. Adding asserts would add noise to what is meant to be a minimal teaching example. The library's own thread_pool::executor_type follows the same pattern.
There was a problem hiding this comment.
@sgerbino, understood—that makes sense for a teaching example. I appreciate the clarification about the semiregular requirement and the consistency with thread_pool::executor_type. Keeping the example minimal is the right call here.
✏️ Learnings added
Learnt from: sgerbino
Repo: cppalliance/capy PR: 195
File: example/custom-executor/custom_executor.cpp:86-108
Timestamp: 2026-03-03T18:46:58.770Z
Learning: In the capy library, executor types (like `run_loop::executor_type` and `thread_pool::executor_type`) can be default-constructed with a null internal pointer to satisfy the `semiregular` requirement of the Executor concept, creating a "valid but unassociated" state. Methods like `context()`, `dispatch()`, and `post()` may assume the executor is properly initialized and not guard against the null state.
Learnt from: sgerbino
Repo: cppalliance/capy PR: 195
File: example/custom-executor/custom_executor.cpp:86-108
Timestamp: 2026-03-03T18:46:58.770Z
Learning: Teaching examples in the capy codebase should be kept minimal without defensive assertions or precondition guards, even when those guards would prevent undefined behavior in edge cases. The focus is on demonstrating the primary usage pattern clearly.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: cppalliance/capy PR: 0
File: .cursor/rules/doc-concept.mdc:0-0
Timestamp: 2026-02-01T22:20:18.267Z
Learning: Applies to **/*.{hpp,h} : Concept javadoc should include par Example with a code block showing a generic algorithm templated on the concept, demonstrating typical usage with destructuring, error handling, and interaction with related concepts
Learnt from: CR
Repo: cppalliance/capy PR: 0
File: .cursor/rules/doc-general.mdc:0-0
Timestamp: 2026-02-01T22:20:39.250Z
Learning: Applies to **/*.{h,hpp,hxx} : Include par Example in function javadocs with one or more code blocks; provide more examples for functions with multiple usage patterns or overloads
| unsigned short port = 8080; | ||
| if (argc > 1) | ||
| port = static_cast<unsigned short>(std::atoi(argv[1])); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file
fd -t f "echo_server.cpp" --exec wc -l {} \;Repository: cppalliance/capy
Length of output: 108
🏁 Script executed:
# Read the specific lines mentioned in the review
sed -n '75,85p' example/echo-server-corosio/echo_server.cppRepository: cppalliance/capy
Length of output: 303
🏁 Script executed:
# Get broader context around lines 79-82 to understand the full function
sed -n '1,100p' example/echo-server-corosio/echo_server.cpp | head -100Repository: cppalliance/capy
Length of output: 2104
Harden CLI port parsing and range validation.
Line 81 uses std::atoi and then narrows to unsigned short without validation; invalid, negative, or out-of-range input can silently produce unintended ports.
💡 Proposed fix
+#include <charconv>
+#include <cstring>
+#include <limits>
@@
unsigned short port = 8080;
if (argc > 1)
- port = static_cast<unsigned short>(std::atoi(argv[1]));
+ {
+ unsigned int parsed = 0;
+ const char* first = argv[1];
+ const char* last = first + std::strlen(first);
+ auto [ptr, ec] = std::from_chars(first, last, parsed);
+ if (ec != std::errc{} || ptr != last ||
+ parsed > std::numeric_limits<unsigned short>::max())
+ {
+ std::cerr << "Invalid port: " << argv[1] << "\n";
+ return 1;
+ }
+ port = static_cast<unsigned short>(parsed);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| unsigned short port = 8080; | |
| if (argc > 1) | |
| port = static_cast<unsigned short>(std::atoi(argv[1])); | |
| unsigned short port = 8080; | |
| if (argc > 1) | |
| { | |
| unsigned int parsed = 0; | |
| const char* first = argv[1]; | |
| const char* last = first + std::strlen(first); | |
| auto [ptr, ec] = std::from_chars(first, last, parsed); | |
| if (ec != std::errc{} || ptr != last || | |
| parsed > std::numeric_limits<unsigned short>::max()) | |
| { | |
| std::cerr << "Invalid port: " << argv[1] << "\n"; | |
| return 1; | |
| } | |
| port = static_cast<unsigned short>(parsed); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@example/echo-server-corosio/echo_server.cpp` around lines 79 - 82, The port
parsing currently casts std::atoi result into unsigned short (port) with no
validation; replace this by parsing argv[1] safely (e.g., use strtol or
std::stoi with error checking), verify the parsed value is numeric and in the
valid TCP port range (1–65535) before assigning to unsigned short port, and
handle invalid input by printing an error and exiting or falling back to the
default 8080; update the code paths around the port variable and the argc/argv
handling so negative, non-numeric, or out-of-range values are rejected rather
than silently wrapped.
There was a problem hiding this comment.
Not addressing. This is example code — the focus is on the Capy/Corosio integration, not CLI parsing. Replacing atoi with from_chars + range validation would triple the size of main() for no pedagogical benefit.
There was a problem hiding this comment.
@sgerbino, understood. You're right that example code should stay focused on demonstrating the core library features—Capy/Corosio integration in this case—rather than adding boilerplate for edge cases like CLI validation. I'll keep that in mind for future reviews of example files.
✏️ Learnings added
Learnt from: sgerbino
Repo: cppalliance/capy PR: 195
File: example/echo-server-corosio/echo_server.cpp:79-82
Timestamp: 2026-03-03T18:47:00.898Z
Learning: In example code files (e.g., files in `example/` directories), avoid flagging robustness issues in peripheral code (such as CLI argument parsing, error handling for non-library operations) when the example's purpose is to demonstrate library integration or specific library features. Example code should remain concise and focused on its pedagogical goal, not production-grade hardening of ancillary concerns.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: cppalliance/capy PR: 0
File: .cursor/rules/doc-concept.mdc:0-0
Timestamp: 2026-02-01T22:20:18.267Z
Learning: Applies to **/*.{hpp,h} : Concept javadoc should include par Example with a code block showing a generic algorithm templated on the concept, demonstrating typical usage with destructuring, error handling, and interaction with related concepts
Learnt from: CR
Repo: cppalliance/capy PR: 0
File: .cursor/rules/doc-general.mdc:0-0
Timestamp: 2026-02-01T22:20:39.250Z
Learning: Applies to **/*.{h,hpp,hxx} : Include par Example in function javadocs with one or more code blocks; provide more examples for functions with multiple usage patterns or overloads
|
GCOVR code coverage report https://195.capy.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-03-03 18:07:11 UTC |
Add four new examples (async-mutex, custom-executor, parallel-tasks, strand-serialization) with CMake build files and Antora documentation pages. Rewrite echo-server-corosio to use the real Corosio API. Fix echo_session to take tcp_socket by value to prevent a dangling reference from the detached coroutine. Add catch(...) to on_error handlers to ensure latch signaling for non-std exceptions.
2 participants
Summary by CodeRabbit
New Features
Chores