"Benno Lossin" <los...@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
