On Wed, 8 Oct 2025 20:35:41 GMT, Serguei Spitsyn <[email protected]> wrote:
>> Johan Sjölen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix copyright
>
> src/hotspot/share/classfile/classFileParser.cpp line 3342:
>
>> 3340:
>> 3341: BSMAttributeEntry* entry =
>> iter.reserve_new_entry(bootstrap_method_ref, num_bootstrap_arguments);
>> 3342: guarantee_property(entry != nullptr, "Invalid BootstrapMethods
>> num_bootstrap_methods. The total amount of space reserved for the
>> BootstrapMethod attribute was not sufficient", CHECK);
>
> Nit: This line is too big. It is a good idea to split the message. Also,
> would it better to move this guaranty for `nullptr` into the
> `reserve_new_entry()`?
I think it's best to keep this null check here so that we can have a
specialized error message. In this case, we are reading a classfile, so we're
parsing untrusted input. That's when we might get a null result when attempting
to reserve a new entry.
In the other cases, we're working from trusted data and a
presumed-to-be-correct algorithm. I've added assertions in these cases, as it
shouldn't break.
> src/hotspot/share/oops/bsmAttribute.hpp line 97:
>
>> 95: int _cur_offset;
>> 96: // Current unused offset into BSMAEs bsm-data array
>> 97: int _cur_array;
>
> Nit: The declarations at lines 95, 97 will be more readable if comments above
> declarations trail declarations. The comment should not start with capital.
Do you mean that you want this?
```c++
int _cur_offset; // current unused offset into BSMAEs offset array
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27198#discussion_r2489955815
PR Review Comment: https://git.openjdk.org/jdk/pull/27198#discussion_r2489948444
