On Fri, 5 Feb 2021 10:36:25 GMT, Serguei Spitsyn <[email protected]> wrote:
>> Hi Lin,
>>
>> A week ago you replied that you are accepting the following suggested
>> refactoring:
>>
>> /*
>> * 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) {
>> 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;
>> }
>> }
>>
>> But I still do not see it in the latest updates from you. It feels like
>> there is some misunderstanding and confusion.
>> Could you, please, clear this up?
>>
>> Thanks,
>> Serguei
>
> Also, this method can be refactored to something like this (the check for
> exactly one option argument is needed):
> private int parseHeapDumpCompressionLevel(String option) {
> String[] keyValue = option.split("=");
> if (!keyValue[0].equals("gz")) {
> err.println("Expected option is "gz="");
> return 0;
> }
> if (keyValue.length != 2) {
> err.println("Exactly one argument is expected for option "gz"");
> return 0;
> }
> int gzl = 0;
> String level = keyValue[1];
> try {
> gzl = Integer.parseInt(level);
> } catch (NumberFormatException e) {
> err.println("gz option value not an integer ("+level+")");
> return 0;
> }
> if (gzl < 1 || gzl > 9) {
> err.println("Compression level out of range (1-9): " + level);
> return 0;
> }
> return gzl;
> }
Hi Serguei,
> But I still do not see it in the latest updates from you. It feels like there
> is some misunderstanding and confusion.
Thanks a lot for remainder! Sorry that I forgot to submit the change.
> Also, this method can be refactored to something like this (the check for
> exactly one option argument is needed):
The change looks nice, I will made the change and submit it in a minute.
BRs,
Lin
-------------
PR: https://git.openjdk.java.net/jdk/pull/1712
