On Fri, 29 Jan 2021 04:27:32 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> 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;
> }
> }
Hi @sspitsyn,
Thanks for your suggestion, the parseHeapDumpCompresslevel() inside tested the
gzlevel, but I think your code is much reasonable. I will make it in next
update.
BRs,
Lin
-------------
PR: https://git.openjdk.java.net/jdk/pull/1712
