RFR: 8267341: macos attempt_reserve_memory_at(arg1, arg2, true) failure [v2]

[Gerard Ziemski]

On Thu, 6 Jan 2022 12:42:01 GMT, David Holmes <dholmes at openjdk.org> wrote:

> Unless a change comes under the "trivial" rule (this does not) then yes two reviews are needed. :)
> 
> A couple of nits below but otherwise this seems a reasonable thing for the test to do.

Thank you David!

> test/hotspot/gtest/runtime/test_os.cpp line 367:
> 
>> 365:   // Workaround: try reserving memory with executable=true
>> 366:   // to figure out if that's supported on this macOS version
>> 367:   bool executable = true;
> 
> Doesn't this local interfere with the same named local at line 388? Seems bad style to have two locals with the same name regardless.

I used to have the code in a separate function, where it didn't conflict, and I now think that factoring that check out into a function is actually the way to go here. What do you think?

> test/hotspot/gtest/runtime/test_os.cpp line 369:
> 
>> 367:   bool executable = true;
>> 368:   size_t test_len = 128;
>> 369:   address p_test = (address)os::reserve_memory(test_len, executable);
> 
> Nit: if you just declare `p_test` as a `char*` then no casts needed here or on line 372.

I like that, done.

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

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