On Thu, Aug 14, 2025 at 9:40 AM Andreas Hindborg <a.hindborg@xxxxxxxxxx> wrote:
>
> "Alice Ryhl" <aliceryhl@xxxxxxxxxx> writes:
>
> > On Wed, Aug 13, 2025 at 7:36 PM Andreas Hindborg <a.hindborg@xxxxxxxxxx> wrote:
> >>
> >> "Alice Ryhl" <aliceryhl@xxxxxxxxxx> writes:
> >>
> >> > For your convenience, I already wrote a safe wrapper of kstrtobool for
> >> > an out-of-tree driver. You're welcome to copy-paste this:
> >> >
> >> > fn kstrtobool(kstr: &CStr) -> Result<bool> {
> >> > let mut res = false;
> >> > to_result(unsafe {
> >> > kernel::bindings::kstrtobool(kstr.as_char_ptr(), &mut res) })?;
> >> > Ok(res)
> >> > }
> >>
> >> Thanks, I did one as well today, accepting `&str` instead. The examples
> >> highlight why it is not great:
> >
> > Yeah, well, I think we should still use it for consistency.
> >
> >> /// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
> >> ///
> >> /// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
> >> /// [oO][NnFf] for "on" and "off". Otherwise it will return `Err(EINVAL)`.
> >> ///
> >> /// # Examples
> >> ///
> >> /// ```
> >> /// # use kernel::str::kstrtobool;
> >> ///
> >> /// // Lowercase
> >> /// assert_eq!(kstrtobool("true"), Ok(true));
> >> /// assert_eq!(kstrtobool("tr"), Ok(true));
> >> /// assert_eq!(kstrtobool("t"), Ok(true));
> >> /// assert_eq!(kstrtobool("twrong"), Ok(true)); // <-- 🤷
> >> /// assert_eq!(kstrtobool("false"), Ok(false));
> >> /// assert_eq!(kstrtobool("f"), Ok(false));
> >> /// assert_eq!(kstrtobool("yes"), Ok(true));
> >> /// assert_eq!(kstrtobool("no"), Ok(false));
> >> /// assert_eq!(kstrtobool("on"), Ok(true));
> >> /// assert_eq!(kstrtobool("off"), Ok(false));
> >> ///
> >> /// // Camel case
> >> /// assert_eq!(kstrtobool("True"), Ok(true));
> >> /// assert_eq!(kstrtobool("False"), Ok(false));
> >> /// assert_eq!(kstrtobool("Yes"), Ok(true));
> >> /// assert_eq!(kstrtobool("No"), Ok(false));
> >> /// assert_eq!(kstrtobool("On"), Ok(true));
> >> /// assert_eq!(kstrtobool("Off"), Ok(false));
> >> ///
> >> /// // All caps
> >> /// assert_eq!(kstrtobool("TRUE"), Ok(true));
> >> /// assert_eq!(kstrtobool("FALSE"), Ok(false));
> >> /// assert_eq!(kstrtobool("YES"), Ok(true));
> >> /// assert_eq!(kstrtobool("NO"), Ok(false));
> >> /// assert_eq!(kstrtobool("ON"), Ok(true));
> >> /// assert_eq!(kstrtobool("OFF"), Ok(false));
> >> ///
> >> /// // Numeric
> >> /// assert_eq!(kstrtobool("1"), Ok(true));
> >> /// assert_eq!(kstrtobool("0"), Ok(false));
> >> ///
> >> /// // Invalid input
> >> /// assert_eq!(kstrtobool("invalid"), Err(EINVAL));
> >> /// assert_eq!(kstrtobool("2"), Err(EINVAL));
> >> /// ```
> >> pub fn kstrtobool(input: &str) -> Result<bool> {
> >> let mut result: bool = false;
> >> let c_str = CString::try_from_fmt(fmt!("{input}"))?;
> >>
> >> // SAFETY: `c_str` points to a valid null-terminated C string, and `result` is a valid
> >> // pointer to a bool that we own.
> >> let ret = unsafe { bindings::kstrtobool(c_str.as_char_ptr(), &mut result as *mut bool) };
> >>
> >> kernel::error::to_result(ret).map(|_| result)
> >> }
> >>
> >> Not sure if we should take `CStr` or `str`, what do you think?
> >
> > Using CStr makes sense, since it avoids having the caller perform a
> > useless utf-8 check.
>
> If we re-implement the entire function in rust, we can do the processing
> on a `&str`. That way, we can skip the allocation to enforce null
> termination. At least for this use case. I would rather do a utf8 check
> than allocate and copy.
You can copy to an array on the stack, so allocations aren't necessary
even if the string is not nul-terminated. I don't think we should
duplicate this logic.
And if we do add a Rust method that does not enforce a nul-check, then
I'd say it should use &[u8] as the argument rather than &str. (Or
possibly &Bstr.)
Alice
