Re: [PATCH v2 2/5] rust: pci: resolve IRQ in vector() and embed IrqRequest in IrqVector

From: Danilo Krummrich

Date: Wed Aug 12 2026 - 13:44:49 EST


On Wed Aug 12, 2026 at 6:38 PM CEST, Gary Guo wrote:
> On Wed Aug 12, 2026 at 12:39 AM BST, Danilo Krummrich wrote:
>> pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
>> if index >= self.count.get() {
>> return Err(EINVAL);
>> }
>>
>> - // SAFETY: `index` is within bounds of this registration's allocation, and `self.dev` is
>> - // the device it was allocated from.
>> - Ok(unsafe { IrqVector::new(self.dev, self, index as u32) })
>> + // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
>> + let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), index as u32) };
>> + if irq < 0 {
>> + return Err(Error::from_errno(irq));
>> + }
>
> Correct me if I'm wrong, but I believe that it's impossible for `pci_irq_vector`
> once we have allocated vector and the index is in bounds. (If that's not the
> case, we should ideally fix that instead.)

You are correct, as of now it is unreachable with the index check above.

> So I think we should just `.expect()` on the error in `Into`.

I don't agree with the conclusion; I don't want this code to rely on an
implementation detail of pci_irq_vector(), which (even though unlikely) could
theoretically change.

If we want to remove the redundancy, then we could maybe drop the index check
above.

(I also prefer IrqVector to be a new type over IrqRequest, as it also guarantees
type wise that a valid IrqVector will always transform into a valid IrqRequest.)