On Wed May 7, 2025 at 11:15 AM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <los...@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
