Page MenuHomeFreeBSD

kern/intr: declare interrupt vectors unsigned
ClosedPublic

Authored by ehem_freebsd_m5p.com on Mar 18 2021, 5:06 PM.

Details

Summary

These should never get values large enough for sign to matter, but one
of them becoming negative could cause problems.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

With a handy build complete, now I know of an extra instance. The
access in sys/arm64/arm64/gic_v3.c seems likely to be uneffected.

Hmm, that title was less than wonderful.

ehem_freebsd_m5p.com retitled this revision from kern/intr: declare many variables unsigned to kern/intr: declare interrupt vectors unsigned.Mar 19 2021, 8:42 PM

There actually aren't (yet) many places where this matters. One place it does matter is isrc_alloc_irq() which deals with the situation by having the "maxirqs" variable shadow intr_nirq. Since intr_nirq will never sensibly be negative it seems better for INTR_IRQ_INVALID to be a large positive value and testing whether an interrupt has a valid value be >= intr_nirq.

I note the isrc_irq entry of struct intr_irqsrc is unsigned. This actually effects D29310, since it means the test used needs a slight adjustment. This also allows D29326, which is getting rid of the shadow variable isrc_alloc_irq() has.

Any chance of review of D29327 and D29310? Presently I'm simply waiting doing nothing...

The 1 month anniversary of D29327. I think I've got appropriate people as reviewers, but no action...

I think this change is fine. I will tinderbox and commit it in a couple of days if no one objects.

With respect to D29310, I suspect it's not getting attention because it does indeed seems like a bikeshed. If the table is full, the problem will manifest as drivers not being able to attach, probably during boot, resulting in an unambiguous failure mode. Interrupt allocation isn't a dynamic operation, its performance is not important and doesn't warrant much scrutiny. Much more so for the unlikely case of interrupt allocation _after_ the table has been filled and some interrupts are freed.

This revision is now accepted and ready to land.Apr 19 2021, 9:19 PM
This revision was automatically updated to reflect the committed changes.