Hi Alan, Thanks for the comment. See my comments inlined below.
Here is the updated webrev: http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8240975/webrev.01 On 3/16/20 3:47 AM, Alan Bateman wrote:
The difference between the 2 constructors might not be obvious at the use sites. I'm just wondering if would be better to use static factory methods instead, e.g. the 2-arg constructor could be replaced with a trusted(caller, searchLibPath) method that would make it a lot more obvious in the places where that will eventually be used.
I have similar comment to myself and didn't come up good static factory method names. I give it a try again: what about newNativeLibraries and newNativeLibrariesWithNoAutoUnload?
A small inconsistency is that VM.isSystemDomainLoader is used in constructor whereas the other checks for null and platform class loader (plus app class loader).
Good catch. Revised.
The Main test could use Path.of("classes"). In setup, dir could be a Path and also Path p = Files.createDirectories(...) would allow the Files.move to be a bit more readable.
Adjusted.
I can't quite tell why the test is skipped with -Xcomp but maybe it's just too slow and times out?
Removed. Cut-n-paste error from an existing test.
A small suggestion for NativeLibrariesTest is that loadWithCustomLoader might be a better name to load p.Test with a custom loader. Also noticed libnativeLibrariesTest.c has a 2017 date on it.
Fixed. thanks Mandy
-Alan.
