Skip to content

Commit 944a55b

Browse files
onur-ozkanojeda
authored andcommitted
rust: rbtree: reduce unsafe blocks on pointer derefs
Refactors parts of the get() and find_best_match() traversal logic to minimize the scope of unsafe blocks and avoid duplicating same safety comments. One of the removed comments was also misleading: // SAFETY: `node` is a non-null node... Ordering::Equal => return Some(unsafe { &(*this).value }), as `node` should have been `this`. No functional changes intended; this is purely a safety improvement that reduces the amount of unsafe blocks while keeping all invariants intact. [ Alice writes: "One consequence of creating a &_ to the bindings::rb_node struct means that we assert immutability for the entire struct and not just the rb_left/rb_right fields, but I have verified that this is ok." - Miguel ] Signed-off-by: Onur Özkan <work@onurozkan.dev> Acked-by: Charalampos Mitrodimas <charmitro@posteo.net> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://patch.msgid.link/20251113144547.502-1-work@onurozkan.dev [ Reworded title and replaced `cursor_lower_bound()` with `find_best_match()` in message. - Miguel ] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
1 parent 6c37b68 commit 944a55b

File tree

1 file changed

+15
-12
lines changed

1 file changed

+15
-12
lines changed

rust/kernel/rbtree.rs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -414,14 +414,17 @@ where
414414
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
415415
// point to the links field of `Node<K, V>` objects.
416416
let this = unsafe { container_of!(node, Node<K, V>, links) };
417+
417418
// SAFETY: `this` is a non-null node so it is valid by the type invariants.
418-
node = match key.cmp(unsafe { &(*this).key }) {
419-
// SAFETY: `node` is a non-null node so it is valid by the type invariants.
420-
Ordering::Less => unsafe { (*node).rb_left },
421-
// SAFETY: `node` is a non-null node so it is valid by the type invariants.
422-
Ordering::Greater => unsafe { (*node).rb_right },
423-
// SAFETY: `node` is a non-null node so it is valid by the type invariants.
424-
Ordering::Equal => return Some(unsafe { &(*this).value }),
419+
let this_ref = unsafe { &*this };
420+
421+
// SAFETY: `node` is a non-null node so it is valid by the type invariants.
422+
let node_ref = unsafe { &*node };
423+
424+
node = match key.cmp(&this_ref.key) {
425+
Ordering::Less => node_ref.rb_left,
426+
Ordering::Greater => node_ref.rb_right,
427+
Ordering::Equal => return Some(&this_ref.value),
425428
}
426429
}
427430
None
@@ -498,18 +501,18 @@ where
498501
let this = unsafe { container_of!(node, Node<K, V>, links) };
499502
// SAFETY: `this` is a non-null node so it is valid by the type invariants.
500503
let this_key = unsafe { &(*this).key };
504+
501505
// SAFETY: `node` is a non-null node so it is valid by the type invariants.
502-
let left_child = unsafe { (*node).rb_left };
503-
// SAFETY: `node` is a non-null node so it is valid by the type invariants.
504-
let right_child = unsafe { (*node).rb_right };
506+
let node_ref = unsafe { &*node };
507+
505508
match key.cmp(this_key) {
506509
Ordering::Equal => {
507510
// SAFETY: `this` is a non-null node so it is valid by the type invariants.
508511
best_links = Some(unsafe { NonNull::new_unchecked(&mut (*this).links) });
509512
break;
510513
}
511514
Ordering::Greater => {
512-
node = right_child;
515+
node = node_ref.rb_right;
513516
}
514517
Ordering::Less => {
515518
let is_better_match = match best_key {
@@ -521,7 +524,7 @@ where
521524
// SAFETY: `this` is a non-null node so it is valid by the type invariants.
522525
best_links = Some(unsafe { NonNull::new_unchecked(&mut (*this).links) });
523526
}
524-
node = left_child;
527+
node = node_ref.rb_left;
525528
}
526529
};
527530
}

0 commit comments

Comments
 (0)