On 10/2/19 9:20 AM, Luc Van Oostenryck wrote: > 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). Yes, that's the way that I read the spec also. >> 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 I might have missed that one. I was busy trying to ignore the 580 similar warnings about rcu variables and asn:4, where each warning takes 3 lines, so it really clutters up the sparse output for the kernel build. -- ~Randy
