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