Re: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
From: Gary Guo
Date: Tue Sep 01 2026 - 10:45:02 EST
On Tue Sep 1, 2026 at 2:32 PM BST, Alexandre Courbot wrote:
> On Tue Sep 1, 2026 at 8:08 PM JST, Danilo Krummrich wrote:
>> On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
>>> That makes me wonder, shouldn't we make `index` take a `u32` directly?
>>> If that's what the C API expects, it does make sense to align to it
>>> instead of forcing users to make a potential unneeded conversion if they
>>> already have a u32.
>>
>> I intentionally did not do this, as the common type for an index is usize. Thus,
>> I do not expect anyone to already have a u32, but to already have a usize, e.g.
>> from some iterator.
>>
>> The fact that the C API did pick unsigned int as index type is an implementation
>> detail the abstraction should bother with.
>
> Thanks for the clarification (and Miguel for elaborating - I wasn't
> completely aware of it!). In that case this patch looks correct to me.
>
> Note that there is another `as` right after, in the same method:
>
> if irq < 0 {
> return Err(Error::from_errno(irq));
> }
>
> // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
> Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self) })
>
>
> We could get rid of it by replacing the `if irq < 0` test with:
>
> let irq = u32::try_from(irq).map_err(|_| Error::from_errno(irq))?;
I wonder if we can just change kernel::error::to_result to return `Result<u32>`
instead and have the cast there?
Best,
Gary