On Tue, 9 Feb 2021 08:28:57 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> 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! > > Hi Lin, > Thank you for update. > > test/lib/jdk/test/lib/hprof/parser/Reader.java: > + } else if ((i >>> 8) == 0x1f8b08) { > + // Possible gziped file. > > Could you, please, define a named constant and use it instead of the literal > value 0x1f8b08? > Also, would it make sense to extend the comment to explain about it a little > bit? > > I'm still looking at some files (e.g., HeapHprofBinWriter.java). > > Thanks, > Serguei Dear @sspitsyn, Thanks for your suggestion. > I'm still looking at some files (e.g., HeapHprofBinWriter.java). No problem, please take your time. Appreciated for your effort helping review! BRs, Lin ------------- PR: https://git.openjdk.java.net/jdk/pull/1712