-
Notifications
You must be signed in to change notification settings - Fork 6
Take a copy of capy's intrusive list as an implementation detail. #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis PR introduces a local intrusive container implementation ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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://46.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-01-21 12:07:54 UTC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/corosio/src/detail/intrusive.hpp`:
- Around line 83-101: The splice_back implementation can corrupt the list when
splicing the list into itself; add a self-splice guard at the top of splice_back
by returning immediately if &other == this (before checking other.empty()),
ensuring head_/tail_ are not modified when other is the same object; apply the
identical self-check to the other splice overload in intrusive_list that takes
an intrusive_list& (the other splice method handling head_/tail_ adjustments) so
both paths skip self-splice.
| void | ||
| splice_back(intrusive_list& other) noexcept | ||
| { | ||
| if(other.empty()) | ||
| return; | ||
| if(tail_) | ||
| { | ||
| tail_->next_ = other.head_; | ||
| other.head_->prev_ = tail_; | ||
| tail_ = other.tail_; | ||
| } | ||
| else | ||
| { | ||
| head_ = other.head_; | ||
| tail_ = other.tail_; | ||
| } | ||
| other.head_ = nullptr; | ||
| other.tail_ = nullptr; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against self-splice to avoid list/queue corruption.
Splicing into the same container will currently create cycles and then clear head_/tail_, effectively corrupting the structure. A simple self-check avoids this class of bugs.
🛠️ Proposed fix
void
splice_back(intrusive_list& other) noexcept
{
+ if (this == &other)
+ return;
if(other.empty())
return;
if(tail_)
{
tail_->next_ = other.head_;
other.head_->prev_ = tail_;
tail_ = other.tail_;
}
else
{
head_ = other.head_;
tail_ = other.tail_;
}
other.head_ = nullptr;
other.tail_ = nullptr;
} void
splice(intrusive_queue& other) noexcept
{
+ if (this == &other)
+ return;
if(other.empty())
return;
if(tail_)
tail_->next_ = other.head_;
else
head_ = other.head_;
tail_ = other.tail_;
other.head_ = nullptr;
other.tail_ = nullptr;
}Also applies to: 199-211
🤖 Prompt for AI Agents
In `@src/corosio/src/detail/intrusive.hpp` around lines 83 - 101, The splice_back
implementation can corrupt the list when splicing the list into itself; add a
self-splice guard at the top of splice_back by returning immediately if &other
== this (before checking other.empty()), ensuring head_/tail_ are not modified
when other is the same object; apply the identical self-check to the other
splice overload in intrusive_list that takes an intrusive_list& (the other
splice method handling head_/tail_ adjustments) so both paths skip self-splice.
Summary by CodeRabbit
Release Notes
Refactor
New Features
✏️ Tip: You can customize this high-level summary in your review settings.