On Tue, 15 Oct 2024 00:21:41 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> The fix improves Attch API protocol and implements updated protocol on
>> windows; shared code is ready to implement updated protocol support on other
>> platforms.
>> More detailed explanations on the 1st comment.
>>
>> Testing: tier1,tier2,tier3,tier4,hs-tier5-svc
>> manually tested backward compatibility (old tools can attach to current
>> VMs, current tools can attach to older VMs) on Windows with jdk21u and jdk8u.
>
> Alex Menkov has updated the pull request incrementally with one additional
> commit since the last revision:
>
> renamed JVM_EnqueueOperation2
>
> renamed JVM_EnqueueOperation2 to JVM_EnqueueOperation_v2 for consistency
> (per Serguei request)
I've posted some nits and one questions.
Otherwise, this change looks good to me.
src/hotspot/os/windows/attachListener_windows.cpp line 194:
> 192: }
> 193: const char* arg(int i) const {
> 194: return (i >= 0 && AttachOperation::arg_count_max) ? _arg[i] :
> nullptr;
Nit: Dot is not needed at the end of comment at line 140.
Q: Should the condition at line 194 be modified as below:
```return (i >= 0 && i < AttachOperation::arg_count_max) ? _arg[i] :
nullptr;```
-------------
PR Review: https://git.openjdk.org/jdk/pull/20782#pullrequestreview-2368827594
PR Review Comment: https://git.openjdk.org/jdk/pull/20782#discussion_r1800860402
