I’ll skip the initialization. Thanks.
> On 5 May 2015, at 15:52, Weijun Wang <[email protected]> wrote: > > That's good, but there is no need to assign null in > > Certificate[] certs = null; > > Or, maybe you can add "if (certs != null)" around the loop, but you might not > like an extra indentation. > > --Max > > On 5/5/2015 10:44 PM, Vincent Ryan wrote: >> OK. How about this? >> >> --- a/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java >> +++ b/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java >> @@ -1,5 +1,5 @@ >> /* >> - * Copyright (c) 1999, 2014, Oracle and/or its affiliates. All rights >> reserved. >> + * Copyright (c) 1999, 2015, Oracle and/or its affiliates. All rights >> reserved. >> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. >> * >> * This code is free software; you can redistribute it and/or modify it >> @@ -1642,23 +1642,22 @@ >> Entry entry = entries.get(alias); >> >> >> // certificate chain >> - int chainLen = 1; >> Certificate[] certs = null; >> >> >> if (entry instanceof PrivateKeyEntry) { >> PrivateKeyEntry keyEntry = (PrivateKeyEntry) entry; >> - if (keyEntry.chain == null) { >> - chainLen = 0; >> - } else { >> - chainLen = keyEntry.chain.length; >> - } >> - certs = keyEntry.chain; >> - >> + if (keyEntry.chain != null) { >> + certs = keyEntry.chain; >> + } else { >> + certs = new Certificate[0]; >> + } >> } else if (entry instanceof CertEntry) { >> - certs = new Certificate[]{((CertEntry) entry).cert}; >> + certs = new Certificate[]{((CertEntry) entry).cert}; >> + } else { >> + certs = new Certificate[0]; >> } >> >> >> - for (int i = 0; i < chainLen; i++) { >> + for (int i = 0; i < certs.length; i++) { >> // create SafeBag of Type CertBag >> DerOutputStream safeBag = new DerOutputStream(); >> safeBag.putOID(CertBag_OID); >> >> >> >>> On 5 May 2015, at 15:10, Weijun Wang <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> Anyway it looks redundant and error-prone to maintain the length of an >>> array in a separate variable. >>> >>> --Max >>> >>> On 5/5/2015 8:32 PM, Vincent Ryan wrote: >>>> Replacing the for loop below with a for-each loop on certs would be >>>> fine except that certs can be null. >>>> I could initialize certs with an empty array on each iteration of the >>>> outer loop but it doesn’t seem to gain much overall. >>>> >>>> >>>>> On 4 May 2015, at 13:10, Weijun Wang <[email protected] >>>>> <mailto:[email protected]>> wrote: >>>>> >>>>> 1662 for (int i = 0; i < chainLen; i++) { >>>>> >>>>> >>>>> On 5/4/2015 6:08 PM, Vincent Ryan wrote: >>>>>> Which line? >>>>>> >>>>>>> On 2 May 2015, at 02:22, Weijun Wang <[email protected] >>>>>>> <mailto:[email protected]>> wrote: >>>>>>> >>>>>>> Is it safe to just run for-each on certs (if it's not null)? >>>>>>> >>>>>>> --Max >>>>>>> >>>>>>> On 5/2/2015 6:39 AM, Vincent Ryan wrote: >>>>>>>> Please review this fix to correct the PKCS12 encoding when a >>>>>>>> secret key is being stored in one keystore entry and a >>>>>>>> certificate in another. >>>>>>>> >>>>>>>> Thanks. >>>>>>>> >>>>>>>> >>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8079129 >>>>>>>> Webrev: http://cr.openjdk.java.net/~vinnie/8079129/webrev.00/ >>>>>>>> >>>>>> >>>> >>
