RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v17]

[Lin Zang]

On Tue, 9 Feb 2021 04:27:44 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> Thanks @plummercj @sspitsyn @YaSuenag  a lot for helping review this PR again and again! 
>> I have marked it as ready for push.
>
> Hi Lin,
> 
> Sorry for confusing you but I've not noticed a duplication in your code:
> 
> 1778                 if (cntTokens > 2) {
> 1779                     usage();
> 1780                 } else {
> 1781                     JMap jmap = new JMap();
> 1782                     String filename = "heap.bin";
> 1783                     int gzlevel = 0;
>  . . . . 
> 1794                     if (cntTokens > 2) {
> 1795                         err.println("More than 2 options specified: " + cntTokens);
> 1796                         usage();
> 1797                         return;
> 1798                     }
> 1799                     if (cntTokens == 1) { // first argument could be filename or "gz="
>                              . . . .
> 
> There are two checks for cntTokens > 2. The second check has to be removed.
> Also, a return needs to be added after the line 1779 with "usage();" , so the "else" statement can be removed.
> This has to be refactored to something like this:
> 1778                 if (cntTokens > 2) {
>                              err.println("More than 2 options specified: " + cntTokens);
> 1779                     usage();
>                              return;
> 1780                 }
> 1781                 JMap jmap = new JMap();
> 1782                 String filename = "heap.bin";
> 1783                 int gzlevel = 0;
> 1799                 if (cntTokens == 1) { // first argument could be filename or "gz="
>                          . . . .
> 
> Thanks,
> Serguei

Dear Serguei,

> There are two checks for cntTokens > 2. The second check has to be removed.
> Also, a return needs to be added after the line 1779 with "usage();" , so the "else" statement can be removed.
> This has to be refactored to something like this:

Nice catch and good suggestion. Thanks a lot for point it out!
I think It is no hurry to push this PR, and welcome for any comment.
Thanks!

-------------

PR: https://git.openjdk.java.net/jdk/pull/1712