Re: random.c patch

[Sandy Harris <sandy@storm.ca>]

Karel Kulhavy wrote:
> 
> There are several places where the rotation yields garbage according to ANSI
> C definition when called with 0 bit position argument.
> 
> diff -Pur linux_reference/drivers/char/random.c linux/drivers/char/random.c
> --- linux_reference/drivers/char/random.c       Wed Jul 19 00:58:13 2000
> +++ linux/drivers/char/random.c Sun Dec 17 22:42:59 2000
> @@ -411,7 +411,7 @@
>  #if (!defined (__i386__))
>  extern inline __u32 rotate_left(int i, __u32 word)
>  {
> -       return (word << i) | (word >> (32 - i));
> +       return (word << i) | (word >> ((-i)&31));
> 
If the calling code guarantees 0 < i < 32, then the patch is unnecessary.
In the kernel I have to hand (2.2.6), grepping gives:

	r->input_rotate = j & 31 ;

and then both calls to the function use r->input_rotate as the first argument,
so the guarantee from higher level code seems to be 0 <= i < 32, in which case
your patch seems needed.

On the other hand, why not put the &= inside the function with something
like:

 extern inline __u32 rotate_left(int i, __u32 word)
{
	switch( i &= 31 )	{ /* cheap version of i %= 32 */
		case 0 :
			return word ;
		default :
			return (word << i) | (word >> (32 - i)) ;
	}

or some faster alternative along the lines of:

{
	i &= 31 ;
	return( i ? ((word << i) | (word >> (32 - i))) : word ) ;

This works right for any i. Yours fails for i >= 32 unless you make it:

       return (word << (i&31)) | (word >> ((-i)&31));

Whichever way is fastest is fine, but I'd advocate doing the range
manipulation inside the function in any case. Why trust the caller?
If the code is maintained or modified, you're almost guaranteed to
be called with bad argumantes in some version.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/