svn commit: r330352 - in head/sys/compat/linuxkpi/common: include/linux src

[Bruce Evans]

On Sat, 3 Mar 2018, Ian Lepore wrote:

> On Sat, 2018-03-03 at 22:49 +0200, Konstantin Belousov wrote:
>> ,,,
>> BTW mstosbt() and other functions have weird bracing in the formula. I
>> think that the formula as calculated by the C operator precedence is
>> fine, i.e. multiplication is done before right shift. But the bracing
>> is redundand then, because the '()' pair next to the return () braces by
>> inclusion is tautological.
>>
>
> Oh, on looking closer, it's not a paste-o, I just didn't put the
> closing paren where I had intended to for making it clear the shift
> happens last.  (So almost everyone except me is still going to think a
> fix has redundant parens.)

It is a good example of how redundant parentheses reduce readability.
Redundant parentheses make the non-redundant parentheses hard to see.
The most important ones are for _ms * (x / 500) (where x =
((uint64_t)1 << 63)).  These are needed since the natural expression
_ms * x / 500 would overflow since it is evaluated left to right.  Then
you want to add redundant parentheses for _ms * (x / 500) >> 32,
although both left to right evaluation and operator precedence work
right for that.  Then style(9) requires redundant parentheses for the
return value.  Normally I would parenthesize x * y >> z since I don't
remember the precedence of '>>' and suspect it is broken (*), but here
there are too many other parentheses.

(*) The precedence of '>>' is indeed broken.  It is like a division operator
so should be at the same level or 1 lower.  But it is 2 lower (+ and - are
in between).  So x + y * z means x + (y * z), but x + y >> z means
(x + y) >> z.  So x + (y >> z) needs parentheses, and (x + y) >> z should
be parenthesized since it is surprising that its parentheses are redundant.

Bruce