bitmap: `next_bit`/`next_zero_bit` panic on inputs documented to return `None`

[GeorgeAndrou]

Hello,

While reading rust/kernel/bitmap.rs I noticed that Bitmap::next_bit:

linux/rust/kernel/bitmap.rs

Lines 459 to 479 in 45c13f3

	/// Finds next set bit, starting from `start`.
	///
	/// Returns `None` if `start` is greater or equal to `self.nbits`.
	#[inline]
	pub fn next_bit(&self, start: usize) -> Option<usize> {
	bitmap_assert!(
	start < self.len(),
	"`start` must be < {} was {}",
	self.len(),
	start
	);
	// SAFETY: `_find_next_bit` tolerates out-of-bounds arguments and returns a
	// value larger than or equal to `self.len()` in that case.
	let index = unsafe { bindings::_find_next_bit(self.as_ptr(), self.len(), start) };
	if index >= self.len() {
	None
	} else {
	Some(index)
	}
	}

and Bitmap::next_zero_bit:

linux/rust/kernel/bitmap.rs

Lines 480 to 500 in 45c13f3

	/// Finds next zero bit, starting from `start`.
	/// Returns `None` if `start` is greater than or equal to `self.len()`.
	#[inline]
	pub fn next_zero_bit(&self, start: usize) -> Option<usize> {
	bitmap_assert!(
	start < self.len(),
	"`start` must be < {} was {}",
	self.len(),
	start
	);
	// SAFETY: `_find_next_zero_bit` tolerates out-of-bounds arguments and returns a
	// value larger than or equal to `self.len()` in that case.
	let index = unsafe { bindings::_find_next_zero_bit(self.as_ptr(), self.len(), start) };
	if index >= self.len() {
	None
	} else {
	Some(index)
	}
	}
	}

can panic if CONFIG_RUST_BITMAP_HARDENED is enabled but don't document it using a # Panics section.

Note that the documentation of both functions (next_bit and next_zero_bit) states that None is returned when start >= self.nbits, which is the exact input that panics when the config is enabled.

On the other hand, similar public functions in the same file such as set_bit, set_bit_atomic, clear_bit, and clear_bit_atomic, do include their panic conditions in a # Panics section (following the Linux Kernel Coding Guidelines).

linux/rust/kernel/bitmap.rs

Lines 292 to 316 in 45c13f3

	impl Bitmap {
	/// Set bit with index `index`.
	///
	/// ATTENTION: `set_bit` is non-atomic, which differs from the naming
	/// convention in C code. The corresponding C function is `__set_bit`.
	///
	/// If CONFIG_RUST_BITMAP_HARDENED is not enabled and `index` is greater than
	/// or equal to `self.nbits`, does nothing.
	///
	/// # Panics
	///
	/// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `index` is greater than
	/// or equal to `self.nbits`.
	#[inline]
	pub fn set_bit(&mut self, index: usize) {
	bitmap_assert_return!(
	index < self.len(),
	"Bit `index` must be < {}, was {}",
	self.len(),
	index
	);
	// SAFETY: Bit `index` is within bounds.
	unsafe { bindings::__set_bit(index, self.as_mut_ptr()) };
	}

Is there something I'm missing? Otherwise I can submit a patch adding the missing # Panics sections or removing the bitmap_assert! from these two functions, if an out-of-range start is meant to return None, since _find_next_bit tolerates it by design.

[ojeda]

It does sound like they should have the section, indeed.

The bitmap subsystem Rust source code is a bit idiosyncratic -- I would suggest asking the maintainers of BITMAP API* (Yury et al.) in the mailing list; or even better, just submitting the patch! :)

Thanks!