lkml.org 
[lkml]   [2009]   [Jan]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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





\
 
 \ /
  Last update: 2009-01-03 19:13    [W:0.985 / U:0.120 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site