linux-kernel - Re: [PATCH v2 2/5] rust: maple

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DC8WOHIEAHQD.21VWTH8VI8QG5@kernel.org>
Date: Fri, 22 Aug 2025 13:05:25 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Miguel Ojeda" <miguel.ojeda.sandonis@...il.com>
Cc: "Alice Ryhl" <aliceryhl@...gle.com>, "Andrew Morton"
 <akpm@...ux-foundation.org>, "Liam R. Howlett" <Liam.Howlett@...cle.com>,
 "Lorenzo Stoakes" <lorenzo.stoakes@...cle.com>, "Miguel Ojeda"
 <ojeda@...nel.org>, "Andrew Ballance" <andrewjballance@...il.com>, "Boqun
 Feng" <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <lossin@...nel.org>, "Andreas Hindborg" <a.hindborg@...nel.org>, "Trevor
 Gross" <tmgross@...ch.edu>, <linux-kernel@...r.kernel.org>,
 <maple-tree@...ts.infradead.org>, <rust-for-linux@...r.kernel.org>,
 <linux-mm@...ck.org>
Subject: Re: [PATCH v2 2/5] rust: maple_tree: add MapleTree

On Fri Aug 22, 2025 at 3:40 AM CEST, Miguel Ojeda wrote:
> So, as a rule of thumb, probably we don't want to show `unwrap()`s in
> examples if the code could have been written "properly" instead

I think we never want to use unwrap(). Even in cases where we relly want to
panic the kernel (because we reached a point where otherwise it is impossible
to prevent undefined behavior) we should make the panic() explicit.

I see it fairly often in reviews that an unwrap() sneaked its way into the code
for things that can easily be handled gracefully, such as simple range checks,
etc.

We should probably check if we can get a clippy warning in place for this.

> , but
> `unwrap_err()`s (i.e. error ones) within an `assert!` are likely fine
> if the example would be better with it.

I agree, for obvious reasons unwrap_err() is much less of a concern.

Yet, it might be read as a pointer in the wrong direction, i.e. read as "unwraps
are OK".

If we want to check for a specific error type (or cause) in doc-tests I think we
should just use is_err_and() instead.

For instance, instead of

	assert_eq!(
	    tree.insert(100, the_answer, GFP_KERNEL).unwrap_err().cause,
	    InsertErrorKind::Occupied,
	);

we could also write

	assert!(tree
	    .insert(100, the_answer, GFP_KERNEL)
	    .is_err_and(|e| e.cause == InsertErrorKind::Occupied));

which is exactly the pattern also shown in the example of is_err_and() [1] and
entirely avoid any unwrap() calls.

[1] https://doc.rust-lang.org/std/result/enum.Result.html#method.is_err_and