As Paul said,
it's a big patch ...
In ImageBuilder, Option should take the description as 3rd parameter
instead of overriding description.
Once this is done, Option should not be abstract but takes the
processing as an argument too
(as a lambda). i.e instead of
new Option(true, "--cmds") {
@Override
void process(ImageBuilder task, String opt, String arg)
throws BadArgs {
task.options.cmds = splitPath(arg, File.pathSeparator);
}
@Override
String description() { return "Location of native commands"; }
}
}
it's better to write:
new Option(true, "--cmds", "Location of native commands",
(ImageBuilder task, String opt, String arg) -> {
task.options.cmds = splitPath(arg, File.pathSeparator);
})
the code is ligther, it uses delegation instead of inheritance and
remove a dozen of inner classes on disk.
In ModuleArchive, close() should continue to close the opened input
streams even if one throw an exception,
@Override
public void close() throws IOException {
IOException e = null;
for(InputStream stream : opened) {
try {
stream.close();
}catch(IOException ex) {
if (e == null) {
e = ex;
} else {
e.addSuppressed(ex);
}
}
}
if (e != null) {
throw e;
}
}
In BasicImageWriter, getString can be simplified to:
public String getString(int offset) {
UTF8String utf8 = strings.get(offset);
if(utf8 != null) {
return utf8.toString();
}
return null;
}
also in generatePerfectHash, you don't need to create a Stream to do a
forEach on it:
input.stream().forEach(
you can write
input.forEach(
ImageReader.LinkNode, instead of
@SuppressWarnings("LeakingThisInConstructor"),
the constructor should be private and you should add a static factory
method that
does parent.addChild(linkNode) after calling the constructor.
in JrtFileAttributeView.get() that takes a Class, you forget to create
the JrtFileAttributeViews
with options as 3 parameters (as you do in get that takes a String).
in JrtPath, instead of
boolean isSame = Arrays.equals(this.getResolvedPath(),
((JrtPath)other).getResolvedPath());
return isSame? isSame : jrtfs.isSameFile(this, (JrtPath)other);
the usual code is:
JrtPath path = (JrtPath) other;
return Arrays.equals(this.getResolvedPath(), path.getResolvedPath()) ||
jrtfs.isSameFile(this, path);
and I fully agree with the review of Paul :)
Rémi
On 05/15/2015 05:45 PM, Jim Laskey (Oracle) wrote:
http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/top/
<http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/top/>
http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/hotspot/
<http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/hotspot/>
http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/jdk/
<http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/jdk/>
http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/langtools/
<http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/langtools/>
There are some issues with regard to the hotspot changes, but I’m working with
Coleen and John R. to resolve those.
Cheers,
— Jim
