Hi Yasumasa,

Some comments.

http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/jdk/com/sun/tools/attach/StartManagementAgent.java.udiff.html
-            if (!ex.getMessage().contains("Invalid com.sun.management.jmxremote.port number")) {
+            if (!ex.getMessage().contains("For input string: \"apa\"")) {

  What is the motivation for this change?
  It seems, the original comparison is better.


http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachException.java.html
http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachIncorrectLibrary.java.html
http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachNoEntry.java.html
http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachReturnError.java.html
  37     public void run(CommandExecutor executor)  {
  38         try{
  A space is missed after 'try'.


  It is odd that all test java classes define exactly the same methods: sharedObjectName(), jmx() and cli().
  Would it better to defin a common base class with these methods?


Otherwise, it looks good.
Thank you for taking care about it!

What tests did you run to make sure there are no regressions?

Thanks,
Serguei



On 11/1/17 05:59, Yasumasa Suenaga wrote:
PING: Could you review and sponsor it?

   http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/


Thanks,

Yasumasa


On 2017/09/29 13:24, Yasumasa Suenaga wrote:
Hi all,

If we try to attach invalid JVMTI agent via JVMTI.agent_load dcmd, we
will get "Command executed successfully". However, it implies error in
JVMTIAgentLoadDCmd.
This message is from JCmd.java when jcmd does not receive any output
from target VM.

I think HotSopt/jcmd should return useful error message to users to
understand the cause of failure.

I uploaded webrev for this issue. Could you review it?

   http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/


This change is work fine on Fedora 26 x86_64 as following  jtreg testcases:

   - hotspot/jtreg/serviceability/attach
   - hotspot/jtreg/serviceability/dcmd/jvmti
   - jdk/com/sun/tools/attach

I cannot access JPRT. So I need a sponsor.
(I cannot test it on other platforms.)


Thanks,

Yasumasa



Reply via email to