<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On May 20, 2015, at 9:21 PM, Valerie Peng <<a href="mailto:valerie.peng@oracle.com" class="">valerie.peng@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">Sean,<br class=""><br class="">Could you please review this change? The changes are mostly the same as the prototype in Jake, but I have to make some modification due to the difference in ServiceLoader lookup in OpenJDK (corresponding META-INF/services/java.security.Provider files in each module) and the related makefile change (merge their content into one for the final image build). Then, I adjusted the Provider.configure() method to take a single String argument to be consistent with the "providerarg" option that keytool defined.<br class=""><br class="">In addition, I also made some misc changes, such as configuring the providers inside ProviderConfig instead of ProviderLoader, add back the doPrivileged block to all the provider constructors. I also have second thought on making the switch to privider name (instead of provider class name) in java.security file, so I reverted the changes on that - that SunPKCS11 provider has its name specified in its configuration file, so when ServiceLoader loads the PKCS11 provider, the configuration file has not been passed to it, so the name is not known at that time. Thus, using the class name for the provider list entry seems to fit the flow better. I also updated the default policy for SunPKCS11 provider given its recent change of using sun.misc.<br class=""><br class="">Webrev: <a href="http://cr.openjdk.java.net/~valeriep/7191662/webrev.00/" class="">http://cr.openjdk.java.net/~valeriep/7191662/webrev.00/</a><br class="">CCC: <a href="http://ccc.us.oracle.com/7191662" class="">http://ccc.us.oracle.com/7191662</a><br class=""></div></blockquote><br class=""></div><div><br class=""></div><div>A quick comment on the META-INF/services config files and the makefile. Merging the service config files is temporary until the module system is moving further along.</div><div><br class=""></div><div class="">src/jdk.crypto.ucrypto/solaris/classes/META-INF/services/java.security.Provider.html</div><div class="">and other os-specific service configuration:</div><div class=""> 1 #[solaris]com.oracle.security.ucrypto.UcryptoProvider</div><div class=""><div class=""><br class=""></div></div><div class="">- why is this commented out? Does the makefile uncomment it? It should be simple</div><div class="">concatenation with</div><div class=""><br class=""></div><div class="">The makefile doesn’t seem right though.</div><div class=""><br class=""></div><div class="">make/gensrc/Gensrc-java.naming.gmk.html</div><div class=""><br class=""></div><div class=""><div class=""> 96 jdk.crypto.ec: $(GENSRC_PROVIDER)</div><div class=""> 98 all: jdk.crypto.ec</div></div><div class=""><br class=""></div><div class="">java.naming doesn’t seem an appropriate module to be the main module for containing all service provider config files. I initially propose to use jdk.crypto.ec as the gensrc module as indicated in line 96,98.</div><div class=""><br class=""></div><div class="">You can rename the file to Gensrc-jdk.crypto.ec and update the content..</div><div class=""><br class=""></div><div class="">GENSRC_PROVIDER := $(SUPPORT_OUTPUTDIR)/gensrc/java.naming/META-INF/services/java.security.Provider</div><div class=""><br class=""></div><div class="">GENSRC_PROVIDER is the output file. line 79-89 is building the target list. I think you need another variable to build up the target list but not GENSRC_PROVIDER.</div><div class=""><br class=""></div><div class="">You can reference how Gensrc-jdk.jdi.gmk concatenates the service config for jdk.jdi and dk.hotspot.agent module.</div><div class=""><br class=""></div><div class=""># Filter com.sun.jdi.connect.Connector<br class="">$(SUPPORT_OUTPUTDIR)/gensrc/jdk.jdi/META-INF/services/com.sun.jdi.connect.Connector: \<br class=""> $(JDK_TOPDIR)/src/jdk.jdi/share/classes/META-INF/services/com.sun.jdi.connect.Connector \<br class=""> $(SUPPORT_OUTPUTDIR)/gensrc/jdk.hotspot.agent/_the.sa.services<br class=""> $(process-provider)<br class=""><br class=""># Copy the same service file into jdk.hotspot.agent so that they are kept the same.<br class="">$(JDK_OUTPUTDIR)/modules/jdk.hotspot.agent/META-INF/services/com.sun.jdi.connect.Connector: \<br class=""> $(SUPPORT_OUTPUTDIR)/gensrc/jdk.jdi/META-INF/services/com.sun.jdi.connect.Connector<br class=""> $(install-file)</div><div class=""><br class=""></div><div class="">Mandy</div></body></html>