On Mon, 28 Mar 2022 12:58:20 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> Christian Hagedorn has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 54 commits: >> >> - Updating some comments >> - Cleanup loading dwarf file and add summary >> - Review comments of first pass by Thomas except dwarf file loading >> - Merge branch 'master' into JDK-8242181 >> - Make dwarf tag NOT_PRODUCT >> - Change log_* to log_develop_* and log_warning to log_develop_info >> - Update test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java >> >> Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> >> - Update test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java >> >> Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> >> - Better formatting of trace output >> - some code move and more cleanups >> - ... and 44 more: >> https://git.openjdk.java.net/jdk/compare/efd3967b...5bea4841 > > src/hotspot/share/utilities/elfFile.cpp line 450: > >> 448: if (buf == nullptr) { >> 449: return false; >> 450: } > > I'd move this close to and local to where it is used. > > Also, you seem to repeat the same pattern a lot "NEW_RESOURCE_ARRAY(n), if > error return something". I'd factor this out to an utility function or > utility macro, maybe one where you pass the error return value as macro > parameter. Thomas's comment caught my attention in the email. NEW_RESOURCE_ARRAY aborts the VM on OOM. Use NEW_RESOURCE_ARRAY_RETURN_NULL if you want to continue. ------------- PR: https://git.openjdk.java.net/jdk/pull/7126
