RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]
Serguei Spitsyn
sspitsyn at openjdk.java.net
Wed Jan 27 09:59:47 UTC 2021
On Tue, 26 Jan 2021 12:42:41 GMT, Lin Zang <lzang at openjdk.org> wrote:
> This is used to guard the case cntTokens == 0, otherwise that t.nextToken() will throw ArrayIndexOutOfBoundsException.
The line `String option = t.nextToken();` can be moved to each of two if statements:
if (cntTokens == 2) {
String option = t.nextToken();
. . .
} else if (cntTokens == 1) {
String option = t.nextToken();
. . .
}
Or you may prefer this: `String option = (cntTokens > 0) ? t.nextToken() : null;`
Here it is more important to save on statements depth/indentation.
I did not understand your answer on the code below (it seems the problem existed before your fix):
gzlevel = parseHeapDumpCompressionLevel(option)
if (gzlevel < 1) {
err.println("Can not parse compression level from option "" + option + "".");
if (gzlevel == 0) {
// compression level not in range.
usage();
return;
} else {
out.println("Use "" + option +"" as dumped file name.");
}
} else {
filename = "heap.bin.gz";
}
There is a check for `gzlevel == 0` when it has been already found this is true: `gzlevel < 1.` It just has no logical sense. I think, the code is broken and has to do something like this:
} else if (cntTokens == 1) {
filename = option;
// Try to parse "gz=" option. If failed, treat it as filename.
if (option.startsWith("gz=")) {
gzlevel = parseHeapDumpCompressionLevel(option);
if (gzlevel > 0 && gzlevel <= 9) {
// Got correct gzlevel, use default filename.
filename = "heap.bin.gz";
} else {
err.println("Compression level not in range: "" + option + "".");
usage();
return;
}
}
}
-------------
PR: https://git.openjdk.java.net/jdk/pull/1712
More information about the serviceability-dev
mailing list