> On Jul 1, 2016, at 8:20 AM, Valerie Peng <valerie.p...@oracle.com> wrote: > > Max, > > Please find comments in line. > > On 6/29/2016 8:18 PM, Wang Weijun wrote: >>> 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? > Right, it's under the jarsigner directory. My eyes were crossed. ;) > Is this used still? I didn't find reference to this one.
It is useless after the jdk.security.jarsigner.JarSigner API. I'll remove it. >>> <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? > I only saw -provider not -addprovider? Confused. http://cr.openjdk.java.net/~weijun/8130302/webrev.06/test/sun/security/tools/keytool/i18n.html.html has 53 <li> keytool -list -storepass password -addprovider SUN > Correct, SUN provider is not exported as a provider for ServiceLoader. I > don't have preference to use -providerClass over -addprovider. Whatever is > the expected usage is fine with me. OK. Thanks Max