On Fri, 29 Jan 2021 05:14:31 GMT, Lin Zang <lz...@openjdk.org> wrote:
>> 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
You are right.
So, in my suggestion just replace this block:
if (gzlevel <= 0 || gzlevel > 9) {
err.println("Invalid "gz=" option: " + option);
usage();
return;
}
with:
if (gzlevel == 0) {
usage();
return;
}
-------------
PR: https://git.openjdk.java.net/jdk/pull/1712
