RFR: 8301404: Factor out os::malloc with os::realloc common code, so that we only have 1 code path [v4]

[Gerard Ziemski]

On Mon, 31 Mar 2025 16:57:36 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> @stefank To answer Stefan's question from Slack channel - `NMTPreInit::handle_realloc()` takes handle and returns bool, so I was trying to stay consistent, which is why `os::pre_init_and_check_size()` takes handle and returns size.
>
>> @stefank To answer Stefan's question from Slack channel - `NMTPreInit::handle_realloc()` takes handle and returns bool, so I was trying to stay consistent, which is why `os::pre_init_and_check_size()` takes handle and returns size.
> 
> My comment was along the lines that I thought it would probably be clearer if os::pre_alloc returned a pointer and let the returned size_t be an output parameter instead. Given that the function allocates memory, I'm still not convinced that returning size_t is beneficial for the readability here.
> 
> I posted that comment in the Slack channel because couldn't prioritize looking at this today, but wanted to give a quick answer to the direct ping in that channel.
> 
> Now that you got my attention I think that the rename to `pre_init_and_check_size` lost the hint that it was performing "pre init *allocation*". The "*and* check size" is also an indication that the function does two different, unrelated(?) things. I would need to look closer to give suggestions for cleanups.

> > @stefank To answer Stefan's question from Slack channel - `NMTPreInit::handle_realloc()` takes handle and returns bool, so I was trying to stay consistent, which is why `os::pre_init_and_check_size()` takes handle and returns size.
> 
> My comment was along the lines that I thought it would probably be clearer if os::pre_alloc returned a pointer and let the returned size_t be an output parameter instead. Given that the function allocates memory, I'm still not convinced that returning size_t is beneficial for the readability here.
> 
> I posted that comment in the Slack channel because couldn't prioritize looking at this today, but wanted to give a quick answer to the direct ping in that channel.
> 
> Now that you got my attention I think that the rename to `pre_init_and_check_size` lost the hint that it was performing "pre init _allocation_". The "_and_ check size" is also an indication that the function does two different, unrelated(?) things. I would need to look closer to give suggestions for cleanups.

I see your point, but then we should touch `NMTPreInit::handle_realloc()` as well?

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

PR Comment: https://git.openjdk.org/jdk/pull/24189#issuecomment-2766910380