Re: JDK 9 RFR of 4026465: Provide more byte array constructors for BigInteger

2015-01-05 Thread Joseph D. Darcy
Hi Brian, The new version looks fine. One suggestion to consider: creating a small private helper method to do the len, off, array-length check. (The two copies of the logic are slightly different.) Thanks, -Joe On 1/2/2015 4:09 PM, Brian Burkhalter wrote: Hello all, Thanks for the

Re: JDK 9 RFR of 4026465: Provide more byte array constructors for BigInteger

2015-01-05 Thread Brian Burkhalter
Hi Joe, The logic differs because one of the cases does not allow for zero length whereas the other does. Thanks, Brian On Jan 5, 2015, at 3:58 PM, Joseph D. Darcy joe.da...@oracle.com wrote: The new version looks fine. One suggestion to consider: creating a small private helper method to

Re: JDK 9 RFR of 4026465: Provide more byte array constructors for BigInteger

2015-01-02 Thread Alan Bateman
On 31/12/2014 02:15, joe darcy wrote: Hi Brian, The new changes generally look good. A few comments, for the new code like 291 } else if ((off 0) || (off val.length) || (len 0) || 292((off + len) val.length) || ((off + len) 0)) { 293 throw

Re: JDK 9 RFR of 4026465: Provide more byte array constructors for BigInteger

2015-01-02 Thread Brian Burkhalter
Hello all, Thanks for the comments. A new patch is here: http://cr.openjdk.java.net/~bpb/4026465/webrev.02/ On Dec 30, 2014, at 6:15 PM, joe darcy joe.da...@oracle.com wrote: The new changes generally look good. A few comments, for the new code like 291 } else if ((off 0) || (off

Re: JDK 9 RFR of 4026465: Provide more byte array constructors for BigInteger

2014-12-30 Thread Stephen Colebourne
Just to note that an IndexOutOfBoundsException is mentioned in the text (line 274, 350) but there is no matching @throws clause. The IndexOutOfBoundsException could have a message added. In general, I would expect an offset/length overload for most methods that take an array, so this seems like a

Re: JDK 9 RFR of 4026465: Provide more byte array constructors for BigInteger

2014-12-30 Thread Brian Burkhalter
I’ve added the suggested @throws tags here: http://cr.openjdk.java.net/~bpb/4026465/webrev.01/ Thanks, Brian On Dec 30, 2014, at 2:33 AM, Stephen Colebourne scolebou...@joda.org wrote: Just to note that an IndexOutOfBoundsException is mentioned in the text (line 274, 350) but there is no

Re: JDK 9 RFR of 4026465: Provide more byte array constructors for BigInteger

2014-12-30 Thread joe darcy
Hi Brian, The new changes generally look good. A few comments, for the new code like 291 } else if ((off 0) || (off val.length) || (len 0) || 292((off + len) val.length) || ((off + len) 0)) { 293 throw new IndexOutOfBoundsException(); it is not

Re: JDK 9 RFR of 4026465: Provide more byte array constructors for BigInteger

2014-12-30 Thread Paul Benedict
Please add @since 1.9 to the new constructors. Cheers, Paul On Tue, Dec 30, 2014 at 8:15 PM, joe darcy joe.da...@oracle.com wrote: Hi Brian, The new changes generally look good. A few comments, for the new code like 291 } else if ((off 0) || (off val.length) || (len 0) ||

JDK 9 RFR of 4026465: Provide more byte array constructors for BigInteger

2014-12-29 Thread Brian Burkhalter
Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-4026465 Patch: http://cr.openjdk.java.net/~bpb/4026465/webrev.00/ Should this issue not look to be worth addressing after all, then I propose that it should be resolved as “Won’t Fix” rather than be left