RFR: 8355481: Clean up MHN_copyOutBootstrapArguments [v4]

David Holmes dholmes at openjdk.org
Mon May 19 08:51:52 UTC 2025


On Mon, 19 May 2025 08:04:32 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Edit: **This code is dead** and we're deleting it, I've kept the original PR description so that following the Github conversation below is easier.
>> 
>> Hi,
>> 
>> I'd like to integrate this simplification of the code for this loop.
>> 
>> We used to have:
>> 
>> ```c++
>> if (start < 0) {
>>   for (int pseudo_index = -4; pseudo_index < 0; pseudo_index++) {
>>     if (start == pseudo_index) {
>>       if (start >= end || 0 > pos || pos >= buf->length())  break;
>>       // ...
>>     }
>>     start++;
>>   }
>> }
>> 
>> 
>> That's exactly the same as:
>> 
>> 
>> int min_end = MIN2(0, end);
>> while (-4 <= start && start < min_end) {
>>   if (pos >= buf->length()) break;
>>   // ...
>>   start++;
>> }
>> 
>> 
>> but the latter looks like a conventional loop.
>> 
>> I'd consider this a basic cleanup, which is worth doing in the name of maintainability.
>> 
>> I would have liked to change the `-4` to `-1` into actual names, but I've no clue where those come from. It doesn't seem worth it to change them if they just happen to be a kludge relying on internal details, or something like that.
>> 
>> Testing: GHA Tier1
>
> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add an assert

Looks good! Thanks for the assertion.

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

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24825#pullrequestreview-2849954283


More information about the hotspot-dev mailing list