From: Alice Ryhl <aliceryhl@google.com>
To: "Onur Özkan" <work@onurozkan.dev>
Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com,
gary@garyguo.net, bjorn3_gh@protonmail.com, lossin@kernel.org,
a.hindborg@kernel.org, tmgross@umich.edu, dakr@kernel.org,
mattgilbride@google.com, wedsonaf@gmail.com, daniel@sedlak.dev,
tamird@gmail.com
Subject: Re: [PATCH] rust: rbtree: simplify finding `current` in `remove_current`
Date: Fri, 4 Jul 2025 07:36:16 +0000 [thread overview]
Message-ID: <aGeEcOEYXiLju-Lj@google.com> (raw)
In-Reply-To: <20250704054539.7715-1-work@onurozkan.dev>
On Fri, Jul 04, 2025 at 08:45:39AM +0300, Onur Özkan wrote:
> The previous version used a verbose `match` to get
> `current`, which may be slightly confusing at first
> glance.
>
> This change makes it shorter and more clearly expresses
> the intent: prefer `next` if available, otherwise fall
> back to `prev`.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
> rust/kernel/rbtree.rs | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs
> index 8d978c896747..8f1052552132 100644
> --- a/rust/kernel/rbtree.rs
> +++ b/rust/kernel/rbtree.rs
> @@ -769,18 +769,10 @@ pub fn remove_current(self) -> (Option<Self>, RBTreeNode<K, V>) {
> // the tree cannot change. By the tree invariant, all nodes are valid.
> unsafe { bindings::rb_erase(&mut (*this).links, addr_of_mut!(self.tree.root)) };
>
> - let current = match (prev, next) {
> - (_, Some(next)) => next,
> - (Some(prev), None) => prev,
> - (None, None) => {
> - return (None, node);
> - }
> - };
> -
> (
> - // INVARIANT:
> - // - `current` is a valid node in the [`RBTree`] pointed to by `self.tree`.
> - Some(Self {
> + next.or(prev).map(|current| Self {
> + // INVARIANT:
> + // - `current` is a valid node in the [`RBTree`] pointed to by `self.tree`.
> current,
> tree: self.tree,
> }),
I'm okay with this change, but the INVARIANT: comment usually goes
before the `StructName {` declaration rather than on the field. For
example, what about this?
// INVARIANT:
// - `current` is a valid node in the [`RBTree`] pointed to by `self.tree`.
let cursor = next.or(prev).map(|current| Self {
current,
tree: self.tree,
});
(cursor, node)
Alice
next prev parent reply other threads:[~2025-07-04 7:36 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-04 5:45 [PATCH] rust: rbtree: simplify finding `current` in `remove_current` Onur Özkan
2025-07-04 7:36 ` Alice Ryhl [this message]
2025-07-04 15:14 ` Onur
2025-07-04 22:15 ` Miguel Ojeda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aGeEcOEYXiLju-Lj@google.com \
--to=aliceryhl@google.com \
--cc=a.hindborg@kernel.org \
--cc=alex.gaynor@gmail.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dakr@kernel.org \
--cc=daniel@sedlak.dev \
--cc=gary@garyguo.net \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=mattgilbride@google.com \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tamird@gmail.com \
--cc=tmgross@umich.edu \
--cc=wedsonaf@gmail.com \
--cc=work@onurozkan.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.