Hi Staffan,

On 09/03/2014 02:27 PM, Staffan Larsen wrote:
…aaaand the link: 
http://cr.openjdk.java.net/~sla/8044398-8039173-8044135-jdk8u/webrev.00/

I have just nits, probably not important because this is a backport and it should not differ from the original patch if not really necessary.

jdk/src/share/classes/sun/tools/attach/HotSpotVirtualMachine.java
* readErrorMessage() - you might want to use `BufferedReader br = new BufferedReader(new InputStreamReader(is))` and `String line = br.readLine()` to read the text contents

* L310 Use StringBuilder instead

jdk/test/sun/management/jmxremote/startstop/JMXStartStopTest.java
* L76 - you can use the concise version `for (ObjectName name : names) {...}` of for loop and forget the iterator

The rest is fine. Thumbs up!

-JB-


/Staffan

On 3 sep 2014, at 14:25, Staffan Larsen <[email protected]> wrote:

This is a review for a backport of JDK-8044135 to jdk8u. This fix had a few 
dependencies that also needed backporting, so included in this review are the 
following fixes from JDK9:

• 8044135 Add API to start JMX agent from attach framework (jdk)
• 8039173 Propagate errors from Diagnostic Commands as exceptions in the attach 
framework (jdk)
• 8044398 Attach code should propagate errors in Diagnostic Commands as errors  
(hotspot)

The only significant change from the version in JDK 9 is that startManagementAgent() and 
startLocalManagementAgent() in VirtualMachine.java have "@since 1.8" (instead 
of 1.9).

After speaking with the maintainers and gatekeepers, I plan to push this to the 
jdk8u-hs-dev repository.

Thanks,
/Staffan



Reply via email to