[PATCH v12 1/3] rust: str: add radix prefixed integer parsing functions

Andreas Hindborg posted 3 patches 7 months, 2 weeks ago
There is a newer version of this series
[PATCH v12 1/3] rust: str: add radix prefixed integer parsing functions
Posted by Andreas Hindborg 7 months, 2 weeks ago
Add the trait `ParseInt` for parsing string representations of integers
where the string representations are optionally prefixed by a radix
specifier. Implement the trait for the primitive integer types.

Tested-by: Daniel Gomez <da.gomez@samsung.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 171 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 878111cb77bc..174e70397305 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -573,7 +573,6 @@ macro_rules! c_str {
 }
 
 #[cfg(test)]
-#[expect(clippy::items_after_test_module)]
 mod tests {
     use super::*;
 
@@ -946,3 +945,174 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 macro_rules! fmt {
     ($($f:tt)*) => ( core::format_args!($($f)*) )
 }
+
+/// Integer parsing functions for parsing signed and unsigned integers
+/// potentially prefixed with `0x`, `0o`, or `0b`.
+pub mod parse_int {
+    use crate::prelude::*;
+    use crate::str::BStr;
+    use core::ops::Deref;
+
+    // Make `FromStrRadix` a public type with a private name. This seals
+    // `ParseInt`, that is, prevents downstream users from implementing the
+    // trait.
+    mod private {
+        use crate::str::BStr;
+
+        /// Trait that allows parsing a [`&BStr`] to an integer with a radix.
+        ///
+        /// # Safety
+        ///
+        /// The member functions of this trait must be implemented according to
+        /// their documentation.
+        ///
+        /// [`&BStr`]: kernel::str::BStr
+        // This is required because the `from_str_radix` function on the primitive
+        // integer types is not part of any trait.
+        pub unsafe trait FromStrRadix: Sized {
+            /// The minimum value this integer type can assume.
+            const MIN: Self;
+
+            /// Parse `src` to [`Self`] using radix `radix`.
+            fn from_str_radix(src: &BStr, radix: u32) -> Result<Self, crate::error::Error>;
+
+            /// Return the absolute value of [`Self::MIN`].
+            fn abs_min() -> u64;
+
+            /// Perform bitwise 2's complement on `self`.
+            ///
+            /// Note: This function does not make sense for unsigned integers.
+            fn complement(self) -> Self;
+        }
+    }
+
+    /// Extract the radix from an integer literal optionally prefixed with
+    /// one of `0x`, `0X`, `0o`, `0O`, `0b`, `0B`, `0`.
+    fn strip_radix(src: &BStr) -> (u32, &BStr) {
+        match src.deref() {
+            [b'0', b'x' | b'X', rest @ ..] => (16, rest.as_ref()),
+            [b'0', b'o' | b'O', rest @ ..] => (8, rest.as_ref()),
+            [b'0', b'b' | b'B', rest @ ..] => (2, rest.as_ref()),
+            // NOTE: We are including the leading zero to be able to parse
+            // literal `0` here. If we removed it as a radix prefix, we would
+            // not be able to parse `0`.
+            [b'0', ..] => (8, src),
+            _ => (10, src),
+        }
+    }
+
+    /// Trait for parsing string representations of integers.
+    ///
+    /// Strings beginning with `0x`, `0o`, or `0b` are parsed as hex, octal, or
+    /// binary respectively. Strings beginning with `0` otherwise are parsed as
+    /// octal. Anything else is parsed as decimal. A leading `+` or `-` is also
+    /// permitted. Any string parsed by [`kstrtol()`] or [`kstrtoul()`] will be
+    /// successfully parsed.
+    ///
+    /// [`kstrtol()`]: https://docs.kernel.org/core-api/kernel-api.html#c.kstrtol
+    /// [`kstrtoul()`]: https://docs.kernel.org/core-api/kernel-api.html#c.kstrtoul
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::str::parse_int::ParseInt;
+    /// # use kernel::b_str;
+    ///
+    /// assert_eq!(Ok(0u8), u8::from_str(b_str!("0")));
+    ///
+    /// assert_eq!(Ok(0xa2u8), u8::from_str(b_str!("0xa2")));
+    /// assert_eq!(Ok(-0xa2i32), i32::from_str(b_str!("-0xa2")));
+    ///
+    /// assert_eq!(Ok(-0o57i8), i8::from_str(b_str!("-0o57")));
+    /// assert_eq!(Ok(0o57i8), i8::from_str(b_str!("057")));
+    ///
+    /// assert_eq!(Ok(0b1001i16), i16::from_str(b_str!("0b1001")));
+    /// assert_eq!(Ok(-0b1001i16), i16::from_str(b_str!("-0b1001")));
+    ///
+    /// assert_eq!(Ok(127i8), i8::from_str(b_str!("127")));
+    /// assert!(i8::from_str(b_str!("128")).is_err());
+    /// assert_eq!(Ok(-128i8), i8::from_str(b_str!("-128")));
+    /// assert!(i8::from_str(b_str!("-129")).is_err());
+    /// assert_eq!(Ok(255u8), u8::from_str(b_str!("255")));
+    /// assert!(u8::from_str(b_str!("256")).is_err());
+    /// ```
+    pub trait ParseInt: private::FromStrRadix + TryFrom<u64> {
+        /// Parse a string according to the description in [`Self`].
+        fn from_str(src: &BStr) -> Result<Self> {
+            match src.deref() {
+                [b'-', rest @ ..] => {
+                    let (radix, digits) = strip_radix(rest.as_ref());
+                    // 2's complement values range from -2^(b-1) to 2^(b-1)-1.
+                    // So if we want to parse negative numbers as positive and
+                    // later multiply by -1, we have to parse into a larger
+                    // integer. We choose `u64` as sufficiently large.
+                    //
+                    // NOTE: 128 bit integers are not available on all
+                    // platforms, hence the choice of 64 bits.
+                    let val = u64::from_str_radix(
+                        core::str::from_utf8(digits).map_err(|_| EINVAL)?,
+                        radix,
+                    )
+                    .map_err(|_| EINVAL)?;
+
+                    if val > Self::abs_min() {
+                        return Err(EINVAL);
+                    }
+
+                    if val == Self::abs_min() {
+                        return Ok(Self::MIN);
+                    }
+
+                    // SAFETY: We checked that `val` will fit in `Self` above.
+                    let val: Self = unsafe { val.try_into().unwrap_unchecked() };
+
+                    Ok(val.complement())
+                }
+                _ => {
+                    let (radix, digits) = strip_radix(src);
+                    Self::from_str_radix(digits, radix).map_err(|_| EINVAL)
+                }
+            }
+        }
+    }
+
+    macro_rules! impl_parse_int {
+        ($ty:ty) => {
+            // SAFETY: We implement the trait according to the documentation.
+            unsafe impl private::FromStrRadix for $ty {
+                const MIN: Self = <$ty>::MIN;
+
+                fn from_str_radix(src: &BStr, radix: u32) -> Result<Self, crate::error::Error> {
+                    <$ty>::from_str_radix(core::str::from_utf8(src).map_err(|_| EINVAL)?, radix)
+                        .map_err(|_| EINVAL)
+                }
+
+                fn abs_min() -> u64 {
+                    #[allow(unused_comparisons)]
+                    if Self::MIN < 0 {
+                        1u64 << (Self::BITS - 1)
+                    } else {
+                        0
+                    }
+                }
+
+                fn complement(self) -> Self {
+                    (!self).wrapping_add((1 as $ty))
+                }
+            }
+
+            impl ParseInt for $ty {}
+        };
+    }
+
+    impl_parse_int!(i8);
+    impl_parse_int!(u8);
+    impl_parse_int!(i16);
+    impl_parse_int!(u16);
+    impl_parse_int!(i32);
+    impl_parse_int!(u32);
+    impl_parse_int!(i64);
+    impl_parse_int!(u64);
+    impl_parse_int!(isize);
+    impl_parse_int!(usize);
+}

-- 
2.47.2
Re: [PATCH v12 1/3] rust: str: add radix prefixed integer parsing functions
Posted by Benno Lossin 7 months, 1 week ago
On Tue May 6, 2025 at 3:02 PM CEST, Andreas Hindborg wrote:
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 878111cb77bc..174e70397305 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -573,7 +573,6 @@ macro_rules! c_str {
>  }
>  
>  #[cfg(test)]
> -#[expect(clippy::items_after_test_module)]
>  mod tests {
>      use super::*;
>  
> @@ -946,3 +945,174 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>  macro_rules! fmt {
>      ($($f:tt)*) => ( core::format_args!($($f)*) )
>  }
> +
> +/// Integer parsing functions for parsing signed and unsigned integers
> +/// potentially prefixed with `0x`, `0o`, or `0b`.
> +pub mod parse_int {

Why not make this its own file? It's 172 lines long already.

> +    pub trait ParseInt: private::FromStrRadix + TryFrom<u64> {
> +        /// Parse a string according to the description in [`Self`].
> +        fn from_str(src: &BStr) -> Result<Self> {
> +            match src.deref() {
> +                [b'-', rest @ ..] => {
> +                    let (radix, digits) = strip_radix(rest.as_ref());
> +                    // 2's complement values range from -2^(b-1) to 2^(b-1)-1.
> +                    // So if we want to parse negative numbers as positive and
> +                    // later multiply by -1, we have to parse into a larger
> +                    // integer. We choose `u64` as sufficiently large.
> +                    //
> +                    // NOTE: 128 bit integers are not available on all
> +                    // platforms, hence the choice of 64 bits.
> +                    let val = u64::from_str_radix(
> +                        core::str::from_utf8(digits).map_err(|_| EINVAL)?,
> +                        radix,
> +                    )
> +                    .map_err(|_| EINVAL)?;
> +
> +                    if val > Self::abs_min() {
> +                        return Err(EINVAL);
> +                    }
> +
> +                    if val == Self::abs_min() {
> +                        return Ok(Self::MIN);
> +                    }
> +
> +                    // SAFETY: We checked that `val` will fit in `Self` above.
> +                    let val: Self = unsafe { val.try_into().unwrap_unchecked() };
> +
> +                    Ok(val.complement())

You're allowing to parse `u32` with a leading `-`? I'd expect an error
in that case. Maybe `complement` should be named `negate` and return a
`Result`?

---
Cheers,
Benno

> +                }
> +                _ => {
> +                    let (radix, digits) = strip_radix(src);
> +                    Self::from_str_radix(digits, radix).map_err(|_| EINVAL)
> +                }
> +            }
> +        }
> +    }
Re: [PATCH v12 1/3] rust: str: add radix prefixed integer parsing functions
Posted by Andreas Hindborg 7 months, 1 week ago
"Benno Lossin" <lossin@kernel.org> writes:

> On Tue May 6, 2025 at 3:02 PM CEST, Andreas Hindborg wrote:
>> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
>> index 878111cb77bc..174e70397305 100644
>> --- a/rust/kernel/str.rs
>> +++ b/rust/kernel/str.rs
>> @@ -573,7 +573,6 @@ macro_rules! c_str {
>>  }
>>
>>  #[cfg(test)]
>> -#[expect(clippy::items_after_test_module)]
>>  mod tests {
>>      use super::*;
>>
>> @@ -946,3 +945,174 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>>  macro_rules! fmt {
>>      ($($f:tt)*) => ( core::format_args!($($f)*) )
>>  }
>> +
>> +/// Integer parsing functions for parsing signed and unsigned integers
>> +/// potentially prefixed with `0x`, `0o`, or `0b`.
>> +pub mod parse_int {
>
> Why not make this its own file? It's 172 lines long already.

Sure. I'm really hoping to land this series for this cycle though, so if
it's OK I would move the code next cycle.

>
>> +    pub trait ParseInt: private::FromStrRadix + TryFrom<u64> {
>> +        /// Parse a string according to the description in [`Self`].
>> +        fn from_str(src: &BStr) -> Result<Self> {
>> +            match src.deref() {
>> +                [b'-', rest @ ..] => {
>> +                    let (radix, digits) = strip_radix(rest.as_ref());
>> +                    // 2's complement values range from -2^(b-1) to 2^(b-1)-1.
>> +                    // So if we want to parse negative numbers as positive and
>> +                    // later multiply by -1, we have to parse into a larger
>> +                    // integer. We choose `u64` as sufficiently large.
>> +                    //
>> +                    // NOTE: 128 bit integers are not available on all
>> +                    // platforms, hence the choice of 64 bits.
>> +                    let val = u64::from_str_radix(
>> +                        core::str::from_utf8(digits).map_err(|_| EINVAL)?,
>> +                        radix,
>> +                    )
>> +                    .map_err(|_| EINVAL)?;
>> +
>> +                    if val > Self::abs_min() {
>> +                        return Err(EINVAL);
>> +                    }
>> +
>> +                    if val == Self::abs_min() {
>> +                        return Ok(Self::MIN);
>> +                    }
>> +
>> +                    // SAFETY: We checked that `val` will fit in `Self` above.
>> +                    let val: Self = unsafe { val.try_into().unwrap_unchecked() };
>> +
>> +                    Ok(val.complement())
>
> You're allowing to parse `u32` with a leading `-`? I'd expect an error
> in that case. Maybe `complement` should be named `negate` and return a
> `Result`?

You would get `Err(EINVAL)` in that case, hitting this:

  if val > Self::abs_min() {
      return Err(EINVAL);
  }


Best regards,
Andreas Hindborg
Re: [PATCH v12 1/3] rust: str: add radix prefixed integer parsing functions
Posted by Benno Lossin 7 months, 1 week ago
On Wed May 7, 2025 at 11:15 AM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@kernel.org> writes:
>> On Tue May 6, 2025 at 3:02 PM CEST, Andreas Hindborg wrote:
>>> +    pub trait ParseInt: private::FromStrRadix + TryFrom<u64> {
>>> +        /// Parse a string according to the description in [`Self`].
>>> +        fn from_str(src: &BStr) -> Result<Self> {
>>> +            match src.deref() {
>>> +                [b'-', rest @ ..] => {
>>> +                    let (radix, digits) = strip_radix(rest.as_ref());
>>> +                    // 2's complement values range from -2^(b-1) to 2^(b-1)-1.
>>> +                    // So if we want to parse negative numbers as positive and
>>> +                    // later multiply by -1, we have to parse into a larger
>>> +                    // integer. We choose `u64` as sufficiently large.
>>> +                    //
>>> +                    // NOTE: 128 bit integers are not available on all
>>> +                    // platforms, hence the choice of 64 bits.
>>> +                    let val = u64::from_str_radix(
>>> +                        core::str::from_utf8(digits).map_err(|_| EINVAL)?,
>>> +                        radix,
>>> +                    )
>>> +                    .map_err(|_| EINVAL)?;
>>> +
>>> +                    if val > Self::abs_min() {
>>> +                        return Err(EINVAL);
>>> +                    }
>>> +
>>> +                    if val == Self::abs_min() {
>>> +                        return Ok(Self::MIN);
>>> +                    }
>>> +
>>> +                    // SAFETY: We checked that `val` will fit in `Self` above.
>>> +                    let val: Self = unsafe { val.try_into().unwrap_unchecked() };
>>> +
>>> +                    Ok(val.complement())
>>
>> You're allowing to parse `u32` with a leading `-`? I'd expect an error
>> in that case. Maybe `complement` should be named `negate` and return a
>> `Result`?
>
> You would get `Err(EINVAL)` in that case, hitting this:
>
>   if val > Self::abs_min() {
>       return Err(EINVAL);
>   }

Ah, I think I asked this in a previous version already... Thanks.

---
Cheers,
Benno