Re: [PATCH for -tip 4/4] irq: for_each_irq_desc() makes simplify

[KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>]

> 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