Re: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32

[Danilo Krummrich <dakr@kernel.org>]

On 9/1/26 3:32 PM, 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))?;

That's a good suggestion!

> Sophon, if you feel like doing it I think this could improve the patch
> further.

Let's please do that as a separate patch though.

Thanks,
Danilo