Re: Code Review Request, 8152237 Support BigInteger.TWO

2016-03-23 Thread Peter Levart
Ok, here's a test... with just the following change: diff -r 9ea9fb3c0c88 src/java.base/share/classes/java/math/BigInteger.java --- a/src/java.base/share/classes/java/math/BigInteger.java Wed Mar 23 18:24:35 2016 +0100 +++ b/src/java.base/share/classes/java/math/BigInteger.java Wed Mar

Re: Code Review Request, 8152237 Support BigInteger.TWO

2016-03-23 Thread Peter Levart
Hi Xuelei, On 03/23/2016 04:26 AM, Xuelei Fan wrote: Hi, Please review the update for the supporting of BigInteger.TWO: http://cr.openjdk.java.net/~xuelei/8152237/webrev/ BigInteger.valueOf(2) is a common BigInteger value used in binary and cryptography operation calculation. The BigInte

Re: Code Review Request, 8152237 Support BigInteger.TWO

2016-03-23 Thread Attila Szegedi
Sorry for beating the dead horse of ObjectIdentifier.java change, but I’d suggest that if that code is later revisited, it be changed to "first.compareTo(BigInteger.TWO) > 0" instead of “… == 1”. Comparing the return value of compareTo to zero (instead of relying on specific set of return value

Re: Code Review Request, 8152237 Support BigInteger.TWO

2016-03-23 Thread Xuelei Fan
Thanks! Xuelei On 3/23/2016 9:44 PM, Wang Weijun wrote: > >> On Mar 23, 2016, at 7:23 PM, Xuelei Fan wrote: >> >> On 3/23/2016 5:44 PM, Wang Weijun wrote: >>> Then why not fix the 2 bugs in a single changeset? >>> >> Both need spec update approval. As they are completely different spec >> upda

Re: Code Review Request, 8152237 Support BigInteger.TWO

2016-03-23 Thread Wang Weijun
> On Mar 23, 2016, at 7:23 PM, Xuelei Fan wrote: > > On 3/23/2016 5:44 PM, Wang Weijun wrote: >> Then why not fix the 2 bugs in a single changeset? >> > Both need spec update approval. As they are completely different spec > update, better to update in 2 enhancements. > > As you have concerns

Re: Code Review Request, 8152237 Support BigInteger.TWO

2016-03-23 Thread Xuelei Fan
Hi Attila, Good catch about the comparing. I updated the code in my local workspace, and I would ask for code review in another enhancement update soon. Thanks, Xuelei On 3/23/2016 9:15 PM, Attila Szegedi wrote: > Sorry for beating the dead horse of ObjectIdentifier.java change, but > I’d sugge

Re: Code Review Request, 8152237 Support BigInteger.TWO

2016-03-23 Thread Xuelei Fan
On 3/23/2016 5:44 PM, Wang Weijun wrote: > Then why not fix the 2 bugs in a single changeset? > Both need spec update approval. As they are completely different spec update, better to update in 2 enhancements. As you have concerns here, I removed ObjectIdentifier.java from this update. See the

Re: Code Review Request, 8152237 Support BigInteger.TWO

2016-03-23 Thread Wang Weijun
Then why not fix the 2 bugs in a single changeset? --Max > 在 2016年3月23日，17:06，Xuelei Fan 写道： > >> On 3/23/2016 3:34 PM, Wang Weijun wrote: >> >>> On Mar 23, 2016, at 12:48 PM, Xuelei Fan wrote: >>> >>> On 3/23/2016 12:10 PM, Wang Weijun wrote: Only 3 files touched. Are you going to make

Re: Code Review Request 8149017 Delayed provider selection broken inRSA client key exchange

2016-03-23 Thread Xuelei Fan
On 3/23/2016 4:43 PM, Seán Coffey wrote: > Looks ok to me Xuelei - tricky one to test, we should ensure some 3rd > party interoperability testing is run. > Would it make sense to append to the exception message to the debug > message on line 135 ? i.e. >> 132 if (debug != null && Debug.isOn("handsh

Re: Code Review Request, 8152237 Support BigInteger.TWO

2016-03-23 Thread Xuelei Fan
On 3/23/2016 3:34 PM, Wang Weijun wrote: > >> On Mar 23, 2016, at 12:48 PM, Xuelei Fan wrote: >> >> On 3/23/2016 12:10 PM, Wang Weijun wrote: >>> Only 3 files touched. Are you going to make the >>> s/BigInteger.valueOf(2)/BigInteger.TWO/ changes in other files with another >>> bug fix? >>> >> T

Re: Code Review Request 8149017 Delayed provider selection broken inRSA client key exchange

2016-03-23 Thread Seán Coffey
Looks ok to me Xuelei - tricky one to test, we should ensure some 3rd party interoperability testing is run. Would it make sense to append to the exception message to the debug message on line 135 ? i.e. 132 if (debug != null && Debug.isOn("handshake")) { 133 System.out.println("The Cipher provi

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

2016-03-23 Thread Alan Bateman
On 23/03/2016 08:12, Wang Weijun wrote: Now that jake/m3 is in jdk9/dev I assume I will just include the jake-related codes and directly push a single changeset to jdk9/dev. Right? Yes, changes like this this can be pushed via jdk9/dev. In the mean-time, we will continue to work in jigsaw/ja

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

2016-03-23 Thread Wang Weijun
> >> The SunPKCS11 compatibility line will be add in a sub-patch for jake. >> > > You can send me the jake’s delta patch once you push the change to jdk9/dev. I've been busy recently on DRBG and will be back working on this one. Now that jake/m3 is in jdk9/dev I assume I will just include the

Re: Code Review Request, 8152237 Support BigInteger.TWO

2016-03-23 Thread Wang Weijun
> On Mar 23, 2016, at 12:48 PM, Xuelei Fan wrote: > > On 3/23/2016 12:10 PM, Wang Weijun wrote: >> Only 3 files touched. Are you going to make the >> s/BigInteger.valueOf(2)/BigInteger.TWO/ changes in other files with another >> bug fix? >> > There are also uses in security components. I wil