On Fri, 25 Oct 2024 15:09:18 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> Simon Tooke has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> changes from review
>
> src/hotspot/os/bsd/memMapPrinter_macosx.cpp line 120:
>
>> 118:
>> 119: static const char* tagToStr(uint32_t user_tag) {
>> 120: switch (user_tag) {
>
> You could reduce the code and remove duplications with an [x
> macro](https://en.wikipedia.org/wiki/X_macro) if you wanted.
Doing this, I found a copy-paste error, so thanks for the suggestion!
> src/hotspot/os/bsd/memMapPrinter_macosx.cpp line 360:
>
>> 358: break;
>> 359: } else if (retval < (int)sizeof(region_info)) {
>> 360: fatal("proc_pidinfo() returned %d", retval);
>
> I would make this an assert, and return without printing anything in release.
I missed this. I have changed it to the Windows version, where it prints the
error and has an assert(), instead of fatal().
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20953#discussion_r1819186695
PR Review Comment: https://git.openjdk.org/jdk/pull/20953#discussion_r1819188325
