fix: prevent null pointer dereference in rb_tree delete_fixup#952
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #952 +/- ##
==========================================
+ Coverage 95.39% 95.58% +0.19%
==========================================
Files 332 332
Lines 21496 21516 +20
==========================================
+ Hits 20506 20566 +60
+ Misses 990 950 -40 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a potential null pointer dereference in the delete_fixup function of the red-black tree implementation. The fix adds a null check for the parent pointer at the beginning of the fixup loop, preventing undefined behavior when deleting the root node in certain edge cases. The fix follows the same pattern already used in insert_fixup (line 248) and is validated by a new regression test.
Key changes:
- Added null pointer guard in
delete_fixuploop to handle edge case where parent is null - Added regression test that triggers the specific deletion sequence that would have caused the null pointer dereference
Comments suppressed due to low confidence (1)
src/data_structures/rb_tree.rs:382
- The loop invariant comment at line 379 states "node is not the root (so parent is not null)", but the newly added null check on line 372 contradicts this invariant. The comment should be updated to reflect that parent can indeed be null, which is the edge case this fix addresses.
/*
* Loop invariants:
* - node is black (or null on first iteration)
* - node is not the root (so parent is not null)
* - All leaf paths going through parent and node have a
* black node count that is 1 lower than other leaf paths.
*/
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pull Request Template
Description
Prevent null pointer dereference in rb_tree delete_fixup.
Type of change
Please delete options that are not relevant.
Checklist:
cargo clippy --all -- -D warningsjust before my last commit and fixed any issue that was found.cargo fmtjust before my last commit.cargo testjust before my last commit and all tests passed.mod.rsfile within its own folder, and in any parent folder(s).DIRECTORY.mdwith the correct link.COUNTRIBUTING.mdand my code follows its guidelines.Please make sure that if there is a test that takes too long to run ( > 300ms), you
#[ignore]that ortry to optimize your code or make the test easier to run. We have this rule because we have hundreds of
tests to run; If each one of them took 300ms, we would have to wait for a long time.