timer-of: don't use conditional expression with mixed 'void' types

Randy Dunlap reports on the sparse list that sparse warns about this
expression:

        of_irq->percpu ? free_percpu_irq(of_irq->irq, clkevt) :
                free_irq(of_irq->irq, clkevt);

and honestly, sparse is correct to warn.  The return type of
free_percpu_irq() is 'void', while free_irq() returns a 'const void *'
that is the devname argument passed in to the request_irq().

You can't mix a void type with a non-void types in a conditional
expression according to the C standard.  It so happens that gcc seems to
accept it - and the resulting type of the expression is void - but
there's really no reason for the kernel to have this kind of
non-standard expression with no real upside.

The natural way to write that expression is with an if-statement:

        if (of_irq->percpu)
                free_percpu_irq(of_irq->irq, clkevt);
        else
                free_irq(of_irq->irq, clkevt);

which is more legible anyway.

I'm not sure why that timer-of code seems to have this odd pattern.  It
does the same at allocation time, but at least there the types match,
and it makes sense as an expression.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index d8c2bd4391d027acf9f4609153a39514e7043f3a..11ff701ff4bb99076a4e19b9faa10e1bd8229d7b 100644 (file)
--- a/drivers/clocksource/timer-of.c
+++ b/drivers/clocksource/timer-of.c
@@ -25,7 +25,9 @@ static __init void timer_of_irq_exit(struct of_timer_irq *of_irq)
 
        struct clock_event_device *clkevt = &to->clkevt;
 
-       of_irq->percpu ? free_percpu_irq(of_irq->irq, clkevt) :
+       if (of_irq->percpu)
+               free_percpu_irq(of_irq->irq, clkevt);
+       else
                free_irq(of_irq->irq, clkevt);
 }