> On Jun 30, 2016, at 9:39 AM, Valerie Peng <valerie.p...@oracle.com> wrote: > > Hi Max, > > Changes look fine. Just some comments/questions as below. > > <src/java.base/share/classes/sun/security/tools/keytool/Resources.java> > - line 46, fix this {0} as well?
I don't see {0} in keytool/Resources.java. There is one in jarsigner/Resources.java: 46 {"signerClass.is.not.a.signing.mechanism", "{0} is not a signing mechanism"}, You mean it's useless now? > > <test/sun/security/tools/jarsigner/AltProvider.java> > - line 151, maybe it should be calling the createUsingTestJDK? Otherwise the > default is to use the compiler JDK Yes. I don't know this method before. > > <test/sun/security/tools/keytool/i18n.html> > - line 53, better to use -providerClass? Both should work. -addprovider works because SUN is already a loaded provider. -providerclass works because sun.security.provider.Sun is a public class in the same module. I prefer -addprovider because -providerclass is only for legacy providers loaded with reflection. In fact, I noticed that SUN is also not in ServiceLoader.load(Provider.class), which means it is not a service. Is that why you suggest the test load it with -providerclass? Thanks Max > > Thanks, > Valerie > > On 6/28/2016 6:09 PM, Wang Weijun wrote: >> Ping again to security-dev. Anyone can approve it? >> >> The latest webrev is at >> >> http://cr.openjdk.java.net/~weijun/8130302/webrev.06 >> >> Change from webrev.05 [1] is tiny. >> >> Thanks >> Max >> >> [1] http://cr.openjdk.java.net/~weijun/8130302/webrev.06/interdiff.patch.html >> >>> On Jun 16, 2016, at 9:33 AM, Wang Weijun <weijun.w...@oracle.com> wrote: >>> >>> >>>> On Jun 16, 2016, at 7:50 AM, Valerie Peng <valerie.p...@oracle.com> wrote: >>>> >>>> No big difference to me. >>> Good, I'll remove the cast. >>> >>> @security-dev, can someone approve the whole webrev.05? >>> >>> http://cr.openjdk.java.net/~weijun/8130302/webrev.05 >>> >>> Thanks >>> Max >>> >>>> Valerie >>>> >>>> On 6/15/2016 8:40 AM, Wang Weijun wrote: >>>>>> On Jun 15, 2016, at 10:57 PM, Mandy Chung<mandy.ch...@oracle.com> wrote: >>>>>> >>>>>>>> 241 throw (InvalidParameterException) >>>>>>>> >>>>>>>> This cast should not be needed? >>>>>>>> >>>>>>> } catch (UcryptoException ue) { >>>>>>> throw (InvalidParameterException) >>>>>>> new InvalidParameterException("Error using " + configArg). >>>>>>> initCause(ue.getCause()); >>>>>>> } >>>>>>> >>>>>>> initCause() returns Throwable but the method's signature throws >>>>>>> InvalidParameterException. >>>>>>> >>>>>> Perhaps have a local variable for InvalidParameterException exception. >>>>> Valerie, are you OK with this? >>>>> >>>>> --Max >>>>> >>>>>> Mandy >