RE: Another BigInteger bug.

["Boehm, Hans" <hans_boehm at hp dot com>]

It looks to me like this patch basically does two things:

1) It fixes a problem with the shift count in

-    words[--nwords] = rnd.nextInt() >>> (numBits % 32);

This looks clearly wrong to me.  It should be more like (32 - numBits % 32),
except that numBits == 0 needs to be handled corectly, i.e. the whole word
needs to be set to 0 in that case.

2) It changes the implementation to be based on Random.nextBytes() instead
of Random.nextInt().

I think (1) is a huge improvement in correctness but, if I understand the
code correclty, (2) is probably a mild regression in performance.  I'd
prefer a patch that only did (1), but this is clearly a net improvement.

Hans

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Friday, August 17, 2001 10:41 AM
> To: Per Bothner
> Cc: Boehm, Hans; 'java-patches@gcc.gnu.org'
> Subject: Re: Another BigInteger bug.
> 
> 
> Hans> Right shifts of negative numbers by nonzero multiples of 32
> Hans> generally produced the wrong answer.  This seems to be yet
> Hans> another case where the code assumed that left shifting an int by
> Hans> 32 produced 0.
> 
> Per> Oops.  How about the following instead, which avoids the 
> extra test:
> 
> Thanks guys.
> 
> Could one of you review this BigInteger patch?
> 
>     http://gcc.gnu.org/ml/java/2001-08/msg00058.html
> 
> I haven't done anything with BigInteger, so I'm reluctant to approve
> or reject it.
> 
> Tom
>