com/sun/crypto/provider/KeyProtector.java:
187 Key k = kFac.generatePrivate(new PKCS8EncodedKeySpec(plain)); 188 return k; No need to create a local variable "k" anymore. JavaKeyStore.java: 128 byte[] passwordBytes = null; No need to define so early and assigned null. No other comments. > On Aug 3, 2018, at 12:37 AM, Seán Coffey <sean.cof...@oracle.com> wrote: > > All valid comments Max. Changes made. Webrev at > > http://cr.openjdk.java.net/~coffeys/webrev.8208583.v4/webrev/index.html > >> I wonder why DestroyedFailedException was a checked exception, what can we >> do if it's thrown? > I guess we could log a message, but given the limited usage case here, I > don't see a need. I was not clear, I meant I cannot find a proper action to do in this case, thus it might be better to defined as an unchecked exception. Thanks Max > > regards, > Sean. > > > On 02/08/2018 17:05, Weijun Wang wrote: >> KeyProtector.java: >> >> 113 pbeKeySpec.clearPassword(); >> >> You can also put this into the finally block. >> >> 189 Arrays.fill(plain, (byte) 0x00); >> >> Can this be in finally? >> >> JavaKeyStore.java: >> >> 149 Arrays.fill(passwordBytes, (byte) 0x00); >> >> In other cases, you call it in a finally block. Unnecessary here? >> >> (Oops, every comment is about finally.) >> >> KeyProtector.java: >> >> 123 public KeyProtector(byte[] password) >> >> How about just "public KeyProtector(byte[] passwordBytes)"? >> >>> On Aug 2, 2018, at 7:38 PM, Seán Coffey <sean.cof...@oracle.com> wrote: >>> >>> No - no problem at all. Some extra exception handling but probably best for >>> the long run. >> I wonder why DestroyedFailedException was a checked exception, what can we >> do if it's thrown? >> >> Thanks >> Max >> >>> http://cr.openjdk.java.net/~coffeys/webrev.8208583.v3/webrev/index.html >>> >>> regards, >>> Sean. >>> >>> On 02/08/2018 02:13, Weijun Wang wrote: >>>>> 1. >>>>> >>>>> I wasn't able to rename to destroy since that method is reserved for the >>>>> Destroyable interface. I've gone with destroyKey. >>>>> >>>> Sorry I wasn't clear but this is exactly what I meant. SecretKey >>>> implements Destroyable so you don't need to define sKey as PBEKey. Does it >>>> make any problem? >>>> >>>> >
