On Tue, Oct 01, 2019 at 11:16:44PM -0700, Randy Dunlap wrote:
> I don't mind the use of ?: for choosing values, but it seems odd to me
> to use it for calling functions, as in: [from drivers/clocksource/timer-of.c),
> line 28 in Linux 5.4-rc1):
>
> of_irq->percpu ? free_percpu_irq(of_irq->irq, clkevt) :
> free_irq(of_irq->irq, clkevt);
>
> where free_percpu_irq() and free_irq() are function calls but not of exactly
> the same type, so sparse (0.6.1-rc1) doesn't like it:
>
> ../drivers/clocksource/timer-of.c:28:24: error: incompatible types in conditional expression (different types):
> ../drivers/clocksource/timer-of.c:28:24: void
> ../drivers/clocksource/timer-of.c:28:24: void const *
> CC drivers/clocksource/timer-of.o
>
>
> gcc doesn't complain about the ?: usage. Is sparse correct here or is it being
> too strict?
I think sparse is well within its mandate here:
?: is an expression and as such should have a defined type
which requires that both sides matches (with a few subtleties).
Here it's not used as an expression but only for the side effect
of calling the functions (with incompatible types).
> I would have just coded the 2 functions calls as
> if (of_irq->percpu)
> free_percpu_irq(of_irq->irq, clkevt);
> else
> free_irq(of_irq->irq, clkevt);
I would do the same.
BTW, the previous warnings is, IMO, much more worrisome:
timer-of.c:28:55: warning: incorrect type in argument 2 (different address spaces)
timer-of.c:28:55: expected void [noderef] <asn:3> *
timer-of.c:28:55: got struct clock_event_device *clkevt
Cheers.
-- Luc
