Looks fine. One nit, line 231, remove "Certs don't match ".
Thanks Max > On Dec 3, 2014, at 10:26, zaiyao liu <zaiyao....@oracle.com> wrote: > > Hi Max, > > Please help to review the update: > http://cr.openjdk.java.net/~rhalade/8048619/webrev.02/ > > Thanks > Kevin > 于 2014/9/23 15:08, zaiyao liu 写道: >> Hi Max, >> >> Please help to check this >> update:http://sqeweb.us.oracle.com/net/sqenfs-1/export1/comp/jsn/users/kevin1/webrev/8048619/webrev/ >> >> Thanks >> >> Kevin >> 于 2014/9/22 17:10, Wang Weijun 写道: >>> On Sep 22, 2014, at 16:11, zaiyao liu <zaiyao....@oracle.com> wrote: >>> >>>> Hi Max, >>>> >>>> Thanks for review. please review the update: >>>> http://sqeweb.us.oracle.com/net/sqenfs-1/export1/comp/jsn/users/kevin1/webrev/8048619/webrev/, >>>> I will ask Rajan to create open webrev link after you passed. >>> 194 if (a.size() != b.size()) { >>> >>> How about "if (a.size() != 1 || b.size() != 1)"? >>> >>> 239-250 >>> >>> This can be as simple as Arrays.equals(certsA, certsB). >>> >>> I would still like a single compareKeyStore() no matter what the size of >>> the keystore is. >>> >>> --Max >>> >>>> Thanks >>>> >>>> Kevin >>>> 于 2014/9/19 15:34, Wang Weijun 写道: >>>>> In compareCerts(), you should not compare Certificate.toString(), its >>>>> equals() method is more reliable. There is no need to define >>>>> compareKeys() and compareCerts(). Instead, you should try not to repeat >>>>> lines 223-231 on 252-260. >>>>> >>>>> The two calls on lines 204 and 205 have keystore names exchanged but >>>>> keypass order is the same. Also, with two calls it means the comparisons >>>>> of keys and certs are duplicated. The else block on line 206 looks >>>>> strange. Do you mean the size is always 1 here? If so, check it. >>>>> Otherwise, there is no guarantee the aliases appear in the same order. >>>>> >>>>> I would be glad to see a generalized comparison method no matter what the >>>>> keystore size is. >>>>> >>>>> Minor issues: >>>>> >>>>> It will be clear if you divide the constant strings at the beginning into >>>>> 2 parts, one for provider names, and one for algorithms. >>>>> >>>>> Line 143 and 145, there should be spaces around the testCase name. >>>>> >>>>> --Max >>>>> >>>>> On Sep 19, 2014, at 11:04, zaiyao liu <zaiyao....@oracle.com> wrote: >>>>> >>>>>> Hi Max, >>>>>> >>>>>> Can you help to review it? >>>>>> >>>>>> Thanks >>>>>> >>>>>> Kevin >>>>>> 于 2014/9/1 13:25, Wang Weijun 写道: >>>>>>> On vacation now. Can you look for someone else? I will be back in Sep >>>>>>> 17 if you are not in a hurry. >>>>>>> >>>>>>> --Max >>>>>>> >>>>>>> On Sep 1, 2014, at 9:37, zaiyao liu <zaiyao....@oracle.com> wrote: >>>>>>> >>>>>>>> Hi Max, >>>>>>>> >>>>>>>> Please review the code change,the purpose of this fix is implement >>>>>>>> tests that convert PKCS12 keystores to other formats. >>>>>>>> >>>>>>>> Webrev: http://cr.openjdk.java.net/~tyan/kevin/JDK-8048619/webrev01/ >>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8048619 >>>>>>>> >>>>>>>> Thanks >>>>>>>> >>>>>>>> Kevin >> >
