On Tue, May 28, 2024 at 02:03:26PM +0200, Rasmus Villemoes wrote:
> On 28/05/2024 12.37, Mateusz Guzik wrote:
> > I misplaced a closing ) in a patch using unlikely() and broke the
> > comparison, see [1] for context.
> >
> > The fix was:
> > - if (unlikely(abs(count + amount)) >= batch) {
> > + if (unlikely(abs(count + amount) >= batch)) {
> >
> > Neither kernel build with W=1 nor C=1 (smatch) report the problem.
> >
> > Given that this compiles fine and for many cases with the bogus usage
> > it also happens to produce the correct result, I have to suspect there
> > are spots in the kernel with the problem.
> >
> > Either way, sounds like something smatch should be testing for.
>
> Well, yes. A quick and dirty 'git grep' actually reveals something funny:
>
> arch/x86/events/intel/core.c: return unlikely(event->hw.config &
> HSW_IN_TX_CHECKPOINTED) != 0;
> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c: if (unlikely(fd_status &
> FM_FD_STAT_RX_ERRORS) != 0) {
The reason why Smatch doesn't find these is because it strips the
"if (unlikely(foo) != 0)" comparisons to just "if (foo)". The != 0 is a
style choice[1]. If it were >= 0 instead then Smatch would complain. I
could fix/change this but my motivation is low... :/
regards,
dan carpenter
[1] https://staticthinking.wordpress.com/2024/02/20/when-to-use-0/
