Changes look good to me. Mike On Mar 7 2013, at 08:36 , BILL PITTORE wrote:
> I updated the code based on the feedback. To fix the windows symbol name > issue that Dean brought up I added a platform specific function, > buildJniFunctionName. In windows it will properly convert _JNI_OnLoad@8 to > _JNI_OnLoad_libname@8 as well as handle JNI_OnLoad symbol name. > > Fixed copyright headers as well. > > Tested on linux and windows > > Webrevs are here: > > http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.01/ > http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ > > bill > > On 3/6/2013 8:47 AM, Alan Bateman wrote: >> On 05/03/2013 23:05, bill.pitt...@oracle.com wrote: >>> This request is tied to bugid 8005716 that deals with adding support for >>> statically linked JNI libraries. >>> >>> The JEP is here: http://openjdk.java.net/jeps/178 >>> >>> The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 >>> >>> The webrevs are here: >>> >>> http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ >>> http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ >>> >>> The main piece of functionality is to check for the presence of >>> JNI_OnLoad_libname to determine if the library specified by 'libname' has >>> been statically linked into the VM. If the symbol is found, it is assumed >>> that the library is linked in and will not be dynamically loaded. >> Overall I think this is quite good and follows the proposal in the JEP. I >> don't see anything obvious wrong with the logic and everything should just >> work for shared libraries as it does today. I assume you'll run all the >> appropriate tests. >> >> I see Dean's suggestion to add a JVM function to allow for a lookup table >> when the symbol information is available, If you do that then you'll need to >> get the hotspot changes in first. Alternatively, change what you have later >> once the hotspot changes are in.Just on the >> >> As findBuiltJniFunction can locate a built-in or a JNI_OnLoad/OnUnload in a >> library then the function name is probably not quite right (shouldn't have >> "Builtin" in the name). >> >> A minor nit in _findBuiltinLib is that if the OOME path should call >> JNU_ReleaseStringPlatformChars before returning. >> >> There are a few commented out statements in jni_util_md.c that I assume will >> be removed. Also you might want to check the indentation in both >> getProcessHandle implementations as they look like 2-spaces whereas the libs >> uses 4 (although this may be mute if you use a JVM function). >> >> Otherwise I think this is good and I can sponsor the jdk part to this and >> help get it into jdk8/tl. >> >> -Alan >> > > >
