Skip to content

Commit

Permalink
timer-of: don't use conditional expression with mixed 'void' types
Browse files Browse the repository at this point in the history
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>
Linus Torvalds committed Oct 2, 2019
1 parent 5021b91 commit 0f1a7b3
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion drivers/clocksource/timer-of.c
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit 0f1a7b3

Please sign in to comment.