On Thu, 28 Jan 2021 22:33:53 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Lin Zang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fix the logic of using gz= as file name
>
> You added clhsdb testing to ClhsdbDumpheap.java, but I had also suggested
> adding testing to BasicJMapTest.java to test pmap. Can you please add some
> testing there also?
Hi Lin,
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java
Thank you for the update.
The list of possible commands must include one more variant with no arguments:
+ * Possible command:
+ * dumpheap gz=1 file;
+ * dumpheap gz=1;
+ * dumpheap file;
+ * dumpheap
The argument processing is still not right.
Why are there no checks for gzlevel < 0 and gzlevel > 9 ?
I'd suggest the following refactoring below:
/*
* Possible commands:
* dumpheap gz=1 file
* dumpheap gz=1
* dumpheap file
* dumpheap
*
* Use default filename if cntTokens == 0.
* Handle cases with cntTokens == 1 or 2.
*/
if (cntTokens > 2) {
err.println("Too big number of options: " + cntTokens);
usage();
return;
}
if (cntTokens >= 1) { // parse first argument which is
"gz=" option
String option = t.nextToken();
gzlevel = parseHeapDumpCompressionLevel(option);
if (gzlevel <= 0 || gzlevel > 9) {
err.println("Invalid "gz=" option: " + option);
usage();
return;
}
filename = "heap.bin.gz";
}
if (cntTokens == 2) { // parse second argument which is
filename
filename = t.nextToken();
if (filename.startsWith("gz=")) {
err.println("Filename should not start with "gz=":
" + filename);
usage();
return;
}
}
-------------
PR: https://git.openjdk.java.net/jdk/pull/1712
