linux-kernel - Re: [PATCH for -tip 4/4] irq: for_each_irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2f11576a0901012156w30a6b162v96aed31d357929e6@mail.gmail.com>
Date:	Fri, 2 Jan 2009 14:56:23 +0900
From:	"KOSAKI Motohiro" <kosaki.motohiro@...il.com>
To:	"Raja R Harinath" <harinath@...rynot.org>
Cc:	linux-kernel@...r.kernel.org
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.

btw, actually current kernel aready have similar macros.
e.g.

#define for_each_node_with_cpus(node)                   \
        for_each_online_node(node)                      \
                if (nr_cpus_node(node))


Shoud we fixed it too? ;-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/