Code logic flaw in `app/driver/rotary,c

[TerryE]

As part of my pre lua53 cleanup , I am adding -Wall to the lua and drivers directories. In the case of the latter this has thrown up a bug in rotary.c. The macro at L40

#define STATUS_IS_PRESSED(x) ((x & 0x80000000) != 0)

has the parentheses around x in the expansion missing which means that the use in L175 generates

|| (last_status ^ new_status & 0x80000000) != 0)

which is not the same as the intended

|| ((last_status ^ new_status) & 0x80000000) != 0)

because the & operator has a higher precedence than ^. Ditto L176.

I've added the extra guard parenthesis for my cleanup so my next PR will fix this, but perhaps @pjsg could confirm that this is correct.

[pjsg]

You are entirely correct -- there should be parens around the x. However, it doesn't actually make much difference. In the driver, this is used to determine whether it is worth waking up the task to handle a message. The module layer gets it correct (I think). Thus there is no functional impact, but there is a performance impact.

Thanks for fixing.

[TerryE]

I picked up a couple of these in my own code when I turned on -Wall 😝

[TerryE]

fixed in #2836