On 05/04/2016 12:07 PM, tytso@thunk.org wrote:

> it doesn't hit the
> UB case which Jeffrey was concerned about. 

That should be good enough for present purposes....

However, in the interests of long-term maintainability, I
would suggest sticking in a comment or assertion saying 
that ror32(,shift) is never called with shift=0.  This
can be removed if/when bitops.h is upgraded.

There is a track record of compilers doing Bad Things in
response to UB code, including some very counterintuitive
Bad Things.

On Wed, May 04, 2016 at 11:29:57AM -0700, H. Peter Anvin wrote:
>>
>> If bitops.h doesn't do the right thing, we need to
>> fix bitops.h.

Most of the ror and rol functions in linux/bitops.h
should be considered unsafe, as currently implemented.
   http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/bitops.h?id=04974df8049fc4240d22759a91e035082ccd18b4#n103

I don't see anything in the file that suggests any limits
on the range of the second argument.  So at best it is an
undocumented trap for the unwary.  This has demonstrably
been a problem in the past.  The explanation in the attached
fix-rol32.diff makes amusing reading.

Of the eight functions
  ror64, rol64, ror32, rol32, ror16, rol16, ror8, and rol8,
only one of them can handle shifting by zero, namely rol32.
It was upgraded on Thu Dec 3 22:04:01 2015; see the attached
fix-rol32.diff.

I find it very odd that the other seven functions were not
upgraded.  I suggest the attached fix-others.diff would make
things more consistent.

Beware that shifting by an amount >= the number of bits in the
word remains Undefined Behavior.  This should be either documented
or fixed.  It could be fixed easily enough.