On Tue, 19 Nov 2024 00:04:16 GMT, Alex Menkov <amen...@openjdk.org> wrote:

> The fix updates Linux (and server-side of macosx) implementation to support 
> Attach API v2 (shared code and Windows implementation were introduced by 
> #20782)
> 
> 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) with jdk21 and jdk8.

Looks good, thanks, a couple of other notes here in case it helps anyone.


AttachOperation::write_reply(ReplyWriter * writer, jint result, const char* 
message, int message_len) 
is this only called by the write_reply method that passes message_len as 
strlen(stream) ?
The new check for len<0 might not really be useful but does no harm.

The existing "char msg[32]" is a buffer used to write the numeric jint result, 
not the message as the name might suggest.

HotSpotVirtualMachine.java already has writeCommand() that handles VERSION_2, 
so the Linux VirtualMachineImpl can just call it.  Also writeString is already 
there in HotSpotVirtualMachine, so removal from Linux is a good cleanup.


I'm sure we all want to find time for the future conversation about actually 
streaming the output rather than passing a single buffer around. 8-)

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

Marked as reviewed by kevinw (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22223#pullrequestreview-2491720772

Reply via email to