From 7b154402fb9d3f4e0d41ca13f0f0890765d35b73 Mon Sep 17 00:00:00 2001 From: pangzhen1 Date: Tue, 26 Aug 2025 22:09:43 +0800 Subject: [PATCH] sched/irq: Consolidate IRQ bounds checking into IRQ_TO_NDX macro Previously, IRQ bounds checking (irq >= 0 && irq < NR_IRQS) was duplicated across multiple IRQ subsystem functions before calling IRQ_TO_NDX(). This led to code duplication and inconsistency. This patch consolidates all IRQ bounds checking into the IRQ_TO_NDX() macro itself, which now returns -EINVAL for out-of-bounds IRQ numbers. This approach: 1. Eliminates duplicated bounds checking code 2. Ensures consistent error handling across all IRQ functions 3. Simplifies caller code - just check if IRQ_TO_NDX() returns negative 4. Makes the macro behavior more predictable and self-contained Changes: - Modified IRQ_TO_NDX() to check (irq < 0 || irq >= NR_IRQS) and return -EINVAL - Removed redundant IRQ range checks in irq_attach(), irq_attach_thread(), irq_attach_wqueue(), and irqchain_detach() - Simplified error handling to check ndx < 0 after IRQ_TO_NDX() call This consolidation reduces code size and improves maintainability while preserving all existing error checking functionality. Signed-off-by: pangzhen1 --- include/nuttx/irq.h | 12 ++++++++---- sched/irq/irq_attach.c | 11 ++--------- sched/irq/irq_attach_thread.c | 8 +------- sched/irq/irq_attach_wqueue.c | 8 +------- sched/irq/irq_chain.c | 24 +++++++++--------------- 5 files changed, 21 insertions(+), 42 deletions(-) diff --git a/include/nuttx/irq.h b/include/nuttx/irq.h index fb5906fa502..4d5cbb49173 100644 --- a/include/nuttx/irq.h +++ b/include/nuttx/irq.h @@ -78,17 +78,21 @@ #if defined(CONFIG_ARCH_MINIMAL_VECTORTABLE_DYNAMIC) # if defined(CONFIG_ARCH_IRQ_TO_NDX) -# define IRQ_TO_NDX(irq) up_irq_to_ndx(irq) +# define IRQ_TO_NDX(irq) \ + ((irq) < 0 || (irq) >= NR_IRQS ? -EINVAL : up_irq_to_ndx(irq)) # else -# define IRQ_TO_NDX(irq) (g_irqmap[irq] ? g_irqmap[irq] : irq_to_ndx(irq)) +# define IRQ_TO_NDX(irq) \ + ((irq) < 0 || (irq) >= NR_IRQS ? -EINVAL : \ + (g_irqmap[irq] ? g_irqmap[irq] : irq_to_ndx(irq))) # endif # define NDX_TO_IRQ(ndx) g_irqrevmap[ndx] #elif defined(CONFIG_ARCH_MINIMAL_VECTORTABLE) # define IRQ_TO_NDX(irq) \ - (g_irqmap[(irq)] < CONFIG_ARCH_NUSER_INTERRUPTS ? g_irqmap[(irq)] : -EINVAL) + ((irq) < 0 || (irq) >= NR_IRQS ? -EINVAL : \ + (g_irqmap[(irq)] < CONFIG_ARCH_NUSER_INTERRUPTS ? g_irqmap[(irq)] : -EINVAL)) # define NDX_TO_IRQ(ndx) ndx_to_irq(ndx) #else -# define IRQ_TO_NDX(irq) (irq) +# define IRQ_TO_NDX(irq) ((irq) < 0 || (irq) >= NR_IRQS ? -EINVAL : (irq)) # define NDX_TO_IRQ(ndx) (ndx) #endif diff --git a/sched/irq/irq_attach.c b/sched/irq/irq_attach.c index 849f9be9fa4..5dcebc62dcf 100644 --- a/sched/irq/irq_attach.c +++ b/sched/irq/irq_attach.c @@ -115,14 +115,7 @@ int irq_attach(int irq, xcpt_t isr, FAR void *arg) { int ret = OK; #if NR_IRQS > 0 - int ndx = -EINVAL; - irqstate_t flags; - - if (irq >= 0 && irq < NR_IRQS) - { - ndx = IRQ_TO_NDX(irq); - } - + int ndx = IRQ_TO_NDX(irq); if (ndx < 0) { ret = ndx; @@ -134,7 +127,7 @@ int irq_attach(int irq, xcpt_t isr, FAR void *arg) * to the unexpected interrupt handler. */ - flags = spin_lock_irqsave(&g_irqlock); + irqstate_t flags = spin_lock_irqsave(&g_irqlock); if (isr == NULL) { /* Disable the interrupt if we can before detaching it. We might diff --git a/sched/irq/irq_attach_thread.c b/sched/irq/irq_attach_thread.c index 9e2fc5df333..2cad59a4d2d 100644 --- a/sched/irq/irq_attach_thread.c +++ b/sched/irq/irq_attach_thread.c @@ -155,13 +155,7 @@ int irq_attach_thread(int irq, xcpt_t isr, xcpt_t isrthread, FAR void *arg, char arg3[32]; /* isrthread */ char arg4[32]; /* arg */ pid_t pid; - int ndx = -EINVAL; - - if (irq >= 0 && irq < NR_IRQS) - { - ndx = IRQ_TO_NDX(irq); - } - + int ndx = IRQ_TO_NDX(irq); if (ndx < 0) { ret = ndx; diff --git a/sched/irq/irq_attach_wqueue.c b/sched/irq/irq_attach_wqueue.c index 783629b9f01..42db826e579 100644 --- a/sched/irq/irq_attach_wqueue.c +++ b/sched/irq/irq_attach_wqueue.c @@ -175,13 +175,7 @@ int irq_attach_wqueue(int irq, xcpt_t isr, xcpt_t isrwork, FAR struct irq_work_info_s *info; int ret = OK; #if NR_IRQS > 0 - int ndx = -EINVAL; - - if (irq >= 0 && irq < NR_IRQS) - { - ndx = IRQ_TO_NDX(irq); - } - + int ndx = IRQ_TO_NDX(irq); if (ndx < 0) { ret = ndx; diff --git a/sched/irq/irq_chain.c b/sched/irq/irq_chain.c index 76c6e94b331..9b1284847de 100644 --- a/sched/irq/irq_chain.c +++ b/sched/irq/irq_chain.c @@ -211,23 +211,19 @@ int irqchain_attach(int ndx, xcpt_t isr, FAR void *arg) int irqchain_detach(int irq, xcpt_t isr, FAR void *arg) { + int ret = OK; #if NR_IRQS > 0 FAR struct irqchain_s *prev; FAR struct irqchain_s *curr; FAR struct irqchain_s *first; - int ret = -EINVAL; - - if ((unsigned)irq < NR_IRQS) + int ndx = IRQ_TO_NDX(irq); + if (ndx < 0) { - int ndx = IRQ_TO_NDX(irq); - irqstate_t flags; - - if (ndx < 0) - { - return ndx; - } - - flags = spin_lock_irqsave(&g_irqchainlock); + ret = ndx; + } + else + { + irqstate_t flags = spin_lock_irqsave(&g_irqchainlock); if (g_irqvector[ndx].handler == irqchain_dispatch) { @@ -276,9 +272,7 @@ int irqchain_detach(int irq, xcpt_t isr, FAR void *arg) ret = irq_detach(irq); } } +#endif return ret; -#else - return OK; -#endif }