Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

+3 -1
+3 -1
drivers/clocksource/timer-of.c
··· 25 25 26 26 struct clock_event_device *clkevt = &to->clkevt; 27 27 28 - of_irq->percpu ? free_percpu_irq(of_irq->irq, clkevt) : 28 + if (of_irq->percpu) 29 + free_percpu_irq(of_irq->irq, clkevt); 30 + else 29 31 free_irq(of_irq->irq, clkevt); 30 32 } 31 33