On Wed, 7 Jul 2021 00:44:11 GMT, Lin Zang <[email protected]> wrote: >> The current implementation of hprof Reader for testing always prompts "Can >> not decompress the compressed hprof file" when there is error for testing >> gzipped heap dump. This is inaccurate if the gzipped file was decompressed >> successfully but the hprof file format is incorrect. So the inaccurate error >> message could be misleading for issue analysis. >> >> This trivial PR refine the error message by simply print "Can not get stack >> trace from the compressed hprof file", the underlying exception from >> GZIPInputStream() or HprofReader() would give accurate error info. > > Lin Zang has updated the pull request incrementally with two additional > commits since the last revision: > > - Throw exception when decompressed data has incorrect magic number > > Also remove unreachable code after exception is added. > - throw exception when magic number is incorrect in decompressed data
Either deleting the out file needs to be handled correctly in all cases or it should left as-is and fixed in a different RFE that also handles stream closing *try-with-resources?) etc. to clean up the code. Otherwise the actual try/catch changes seem okay. Thanks, David test/lib/jdk/test/lib/hprof/parser/Reader.java line 175: > 173: true, debugLevel); > 174: r.read(); > 175: out.delete(); I thought this was going to handled by the separate RFE to fix the general logic? And you don't delete before throwing below. There should be a finally clause to handle deletion consistently. ------------- Changes requested by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4685
