Hi Henry,
I think you want to revert the order of the following two lines in
ProxyClassDumper:
67 isReadyToUse();
68 dumpDir = tmp;
Otherwise looks good now.
The following is up to you. Just style nits...
If you wanted to be extra user-friendly, you could log all transitions
of "valid dir" -> "invalid dir" and vice versa. Like the following:
private boolean isReadyToUse() {
if (dumpDir == null) {
return false;
}
String errMsg;
if (!Files.exists(dumpDir)) {
errMsg = "Directory " + dumpDir + " does not exist";
} else if (!Files.isDirectory(dumpDir)) {
errMsg = "Path " + dumpDir + " is not a directory";
} else if (!Files.isWritable(dumpDir)) {
errMsg = "Directory " + dumpDir + " is not writable";
} else {
// show info message about re-validated directory
if (invalidDir.getAndSet(false)) {
PlatformLogger.getLogger(ProxyClassesDumper.class.getName())
.info("Directory " + dumpDir +
" is writable directory now -
dumping restored");
}
// validated dump directory, ready to go
return true;
}
// show warning message about invalidated directory
if (! invalidDir.getAndSet(true)) {
PlatformLogger.getLogger(ProxyClassesDumper.class.getName())
.warning(errMsg + " - dumping temporarily
disabled");
}
return false;
}
In constructor's exception handler, you could write:
PlatformLogger.getLogger(ProxyClassesDumper.class.getName())
.warning("Path " + path + " is not valid - dumping
permanently disabled", ex);
You don't need .toString() calls for objects that are part of string
concatenation expressions. It's done automatically for you. It's even
more safe, since it can avoid NPE when any such object is null...
Regards, Peter
On 10/03/2013 01:56 AM, Henry Jen wrote:
Hi,
Please review update of the webrev at
http://cr.openjdk.java.net/~henryjen/ccc/8023524/4/webrev
This update address comments from Remi and Peter,
- Remove the unnecessary static field, also take out not needed
singleton behavior of ProxyClassesDumper
- Ensure InvalidPathException won't stop InnerClassLambdaMetaFactory
- Change test code to create readonly directory that didn't work
properly on Windows.
Cheers,
Henry
