[PATCH v2] rust: bitmap: document panics in `next_bit` and `next_zero_bit`

Georgios Androutsopoulos posted 1 patch 4 weeks, 1 day ago
There is a newer version of this series
rust/kernel/bitmap.rs | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
[PATCH v2] rust: bitmap: document panics in `next_bit` and `next_zero_bit`
Posted by Georgios Androutsopoulos 4 weeks, 1 day ago
`next_bit()` and `next_zero_bit()` use `bitmap_assert!()` to check
that `start` is less than `self.len()`, which panics when
`CONFIG_RUST_BITMAP_HARDENED` is enabled. However, neither function
has a `# Panics` section, and both document that `None` is returned
for exactly the input that triggers the panic.

Update the documentation of `next_bit()` and `next_zero_bit()` to add
the missing `# Panics` sections, add the missing blank doc comment line
before `Returns` in `next_zero_bit()`, and simplify the wording.

Link: https://github.com/Rust-for-Linux/linux/issues/1252
Signed-off-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>
---
Changes in v2:
- Simplify doc wording following feedback from Alice Ryhl.
- Remove extra empty line between Link: and Signed-off-by:.
- Link to v1: https://lore.kernel.org/rust-for-linux/20260827150719.109145-1-georgeandrout13@gmail.com/
---
 rust/kernel/bitmap.rs | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
index b27e0ec80..b51c8a96f 100644
--- a/rust/kernel/bitmap.rs
+++ b/rust/kernel/bitmap.rs
@@ -459,7 +459,13 @@ pub fn last_bit(&self) -> Option<usize> {
 
     /// Finds next set bit, starting from `start`.
     ///
-    /// Returns `None` if `start` is greater or equal to `self.nbits`.
+    /// Returns `None` if no bits are set on or after the given index. The
+    /// index `start` must be in bounds.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `CONFIG_RUST_BITMAP_HARDENED` is enabled and `start` is
+    /// out of bounds.
     #[inline]
     pub fn next_bit(&self, start: usize) -> Option<usize> {
         bitmap_assert!(
@@ -479,7 +485,14 @@ pub fn next_bit(&self, start: usize) -> Option<usize> {
     }
 
     /// Finds next zero bit, starting from `start`.
-    /// Returns `None` if `start` is greater than or equal to `self.len()`.
+    ///
+    /// Returns `None` if no bits are zero on or after the given index. The
+    /// index `start` must be in bounds.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `CONFIG_RUST_BITMAP_HARDENED` is enabled and `start` is
+    /// out of bounds.
     #[inline]
     pub fn next_zero_bit(&self, start: usize) -> Option<usize> {
         bitmap_assert!(

base-commit: 73e3f0710014fe6d4ed98cfc02292f6121db7558
-- 
2.47.3
Re: [PATCH v2] rust: bitmap: document panics in `next_bit` and `next_zero_bit`
Posted by Alexandre Courbot 3 weeks, 5 days ago
On Fri Aug 28, 2026 at 10:35 PM JST, Georgios Androutsopoulos wrote:
> `next_bit()` and `next_zero_bit()` use `bitmap_assert!()` to check
> that `start` is less than `self.len()`, which panics when
> `CONFIG_RUST_BITMAP_HARDENED` is enabled. However, neither function
> has a `# Panics` section, and both document that `None` is returned
> for exactly the input that triggers the panic.
>
> Update the documentation of `next_bit()` and `next_zero_bit()` to add
> the missing `# Panics` sections, add the missing blank doc comment line
> before `Returns` in `next_zero_bit()`, and simplify the wording.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1252

Should this be `Closes:` instead of `Link:`?

> Signed-off-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>

As I mentioned earlier I hope we can stop panicking on these [1], but
meanwhile it's indeed a good idea to have a Panic section.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

[1] https://lore.kernel.org/DKUKM1RPFDA7.1MBU5EY40OJ4J@nvidia.com
Re: [PATCH v2] rust: bitmap: document panics in `next_bit` and `next_zero_bit`
Posted by Miguel Ojeda 3 weeks, 4 days ago
On Mon, Aug 31, 2026 at 3:08 AM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> Should this be `Closes:` instead of `Link:`?

Maybe -- it depends on whether we consider this kind of documentation
hole a bug or not. But if we do, then we want a Fixes: too.

Cheers,
Miguel
Re: [PATCH v2] rust: bitmap: document panics in `next_bit` and `next_zero_bit`
Posted by George Androutsopoulos 3 weeks, 4 days ago
On Tue, Sep 1, 2026 at 5:48 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
> Maybe -- it depends on whether we consider this kind of documentation
> hole a bug or not. But if we do, then we want a Fixes: too.

I lean towards treating that as a bug and sending a v3 with `Closes:`
and `Fixes:`.

These are public `kernel` crate functions intended for driver authors,
and nothing in their documentation indicated they could panic. Finding
that out required reading the body and following it into
`bitmap_assert!`. For `next_bit()` and `next_zero_bit()` the docs went
further and stated that `None` is returned for exactly the input that
panics when `CONFIG_RUST_BITMAP_HARDENED` is enabled.

The same question applies to the `id_pool` patch ("rust: id_pool:
document panics in `find_unused_id` and `release_id`"), where the
docs were silent rather than contradictory.

Thanks,
Georgios
Re: [PATCH v2] rust: bitmap: document panics in `next_bit` and `next_zero_bit`
Posted by Miguel Ojeda 3 weeks, 4 days ago
On Tue, Sep 1, 2026 at 5:09 PM George Androutsopoulos
<georgeandrout13@gmail.com> wrote:
>
> These are public `kernel` crate functions intended for driver authors,
> and nothing in their documentation indicated they could panic.

I tend to agree, especially since we have treated even documentation
typos (and especially so if they are public items, i.e. rendered in
the actual docs) as fixes before (not typos on mere comments, of
course).

But it is also reasonable to say they aren't, since they don't affect
the end user (directly, at least).

Either way, it sounds OK :) Bitmap will decide when they pick it up
(unless they want me to take it -- I will be sending a rust-fixes PR
anyway, so I could).

Thanks!

Cheers,
Miguel