Messages in this thread Patch in this message | | | Date | Sun, 4 Jan 2009 03:11:05 +0900 (JST) | From | KOSAKI Motohiro <> | Subject | Re: [PATCH for -tip 4/4] irq: for_each_irq_desc() makes simplify |
| |
> Hi > > >> # define for_each_irq_desc(irq, desc) \ > >> for (irq = 0, desc = irq_to_desc(irq); irq < nr_irqs; \ > >> - irq++, desc = irq_to_desc(irq)) > >> + irq++, desc = irq_to_desc(irq)) \ > >> + if (desc) > >> + > >> + > >> # define for_each_irq_desc_reverse(irq, desc) \ > >> for (irq = nr_irqs - 1, desc = irq_to_desc(irq); irq >= 0; \ > >> - irq--, desc = irq_to_desc(irq)) > >> + irq--, desc = irq_to_desc(irq)) \ > >> + if (desc) > > > > I know this has gone in, but isn't this naked 'if' unsafe. Consider the > > following hypothetical code: > > > > if (safe) > > for_each_irq_desc(irq, desc) { > > ... > > } > > else > > panic(); > > > > With the macro definition above, the loop would panic() each time !desc, > > and _not_ panic() when !safe. I'd consider this behaviour to be > > unexpected, to say the least :-) > > Correct. > > > The fix is to change the > > > > if (desc) > > > > in the macro to > > > > if (!desc) ; else > > Ok. I'll do that. > Very thanks for good reviewing.
Done. Thnaks Raja.
Ingo, could you please apply this patch to -tip tree?
=== From 6f1210e6912afd8ca07518e82f89ba92137302b2 Mon Sep 17 00:00:00 2001 From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> Date: Sun, 4 Jan 2009 02:37:26 +0900 Subject: [PATCH] sparseirq: fix for_each_irq_desc() nit
Raja reported for_each_irq_desc() has possibility unsafeness. if anyone write folliwing code, for_each_irq_desc() doesn't works intetionally. (Now, its code doesn't exist at all)
if (safe) for_each_irq_desc(irq, desc) { ... } else panic();
Reported-by: Raja R Harinath <harinath@hurrynot.org> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> CC: Ingo Molnar <mingo@elte.hu> CC: Yinghai Lu <yinghai@kernel.org> --- include/linux/irqnr.h | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/irqnr.h b/include/linux/irqnr.h index 5504a5c..99b91c6 100644 --- a/include/linux/irqnr.h +++ b/include/linux/irqnr.h @@ -23,13 +23,17 @@ extern struct irq_desc *irq_to_desc(unsigned int irq); # define for_each_irq_desc(irq, desc) \ for (irq = 0, desc = irq_to_desc(irq); irq < nr_irqs; \ irq++, desc = irq_to_desc(irq)) \ - if (desc) + if (!desc) \ + ; \ + else # define for_each_irq_desc_reverse(irq, desc) \ for (irq = nr_irqs - 1, desc = irq_to_desc(irq); irq >= 0; \ irq--, desc = irq_to_desc(irq)) \ - if (desc) + if (!desc) \ + ; \ + else #endif /* CONFIG_GENERIC_HARDIRQS */ -- 1.5.3
| |