On Wed, 4 Jan 2023 09:22:40 GMT, Yi Yang <yy...@openjdk.org> wrote:
>> harmless refactor to share code across different platforms of
>> VirtualMachineImpl:
>> 1. Shared code to process command response after requesting a command
>> execution
>> 2. Read functionality in SocketInputStream can be reused
>
> Yi Yang has refreshed the contents of this pull request, and previous commits
> have been removed. The incremental views will show differences compared to
> the previous content of the PR. The pull request contains two new commits
> since the last revision:
>
> - separate renaming
> - 8299518: HotSpotVirtualMachine shared code across different platforms
src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java line 195:
> 193: * InputStream for the socket connection to get target VM
> 194: */
> 195: private static class SocketInputStreamImpl extends SocketInputStream
> {
Can this class definition also be shared by making it a protected nested class
in the superclass?
src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java line
188:
> 186:
> 187: // known error
> 188: private static final int ATTACH_ERROR_BADVERSION = 101;
It doesn't look right that this has the same value as ATTACH_ERROR_NOTONCP
src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java line
372:
> 370: }
> 371:
> 372: void processCompletionStatus(IOException ioe, String cmd,
> InputStream sis) throws AgentLoadException, IOException {
Some doc comments for this method would be good.
src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java line
425:
> 423:
> 424: protected abstract int readImpl(long fd, byte[] bs, int off, int
> len) throws IOException;
> 425: protected abstract void closeImpl(long fd) throws IOException;
If the subclasses all override these in exactly the same way then these do not
need to be abstract and can simply delegate to VirtualMachineImpl.xxx
-------------
PR: https://git.openjdk.org/jdk/pull/11823
