Hi Christian,
Thank you for looking at it!
On 6/21/16 04:56, Christian Tornqvist wrote:
Hi Serguei,
I’ve only looked at the test code so far, here are some comments:
* You’re not allowed to call System.exit in a jtreg test. If something
fails, you need to throw an exception.
Ok, an exception is Ok with me.
* Instead of a run() method you could simply collapse all this into
the main method like this:
assertEQ(check(), 0);
Will check if this works for me.
* Would it make sense to throw exceptions with useful error messages
from the JNI code instead of returning failed status codes? Our error
matching system would work a lot better with unique error messages.
Will check if this works well in this case.
The unique error messages are already printed in the native code.
Thanks,
Serguei
Thanks,
Christian
*From:*serguei.spit...@oracle.com [mailto:serguei.spit...@oracle.com]
*Sent:* Tuesday, June 21, 2016 6:49 AM
*To:* Alan Bateman <alan.bate...@oracle.com>;
serviceability-dev@openjdk.java.net; Christian Törnqvist
<christian.tornqv...@oracle.com>; Alexander Kulyakhtin
<alexander.kulyakh...@oracle.com>; hotspot-...@openjdk.java.net
Developers <hotspot-...@openjdk.java.net>
*Subject:* Re: Jigsaw Enhancement RFR: 8159145 Add JVMTI function
GetModuleByPackageName
On 6/21/16 03:23, Alan Bateman wrote:
On 21/06/2016 10:54, serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com> wrote:
Please, review the Jigsaw fix for the enhancement:
https://bugs.openjdk.java.net/browse/JDK-8159145
The Hotspot webrev:
http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/
The Jdk webrev:
http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/
Serguei - can you put the patches on cr.openjdk.java.net?
Sorry.
My initial plan was to write an nsk.jvmti test and review it on a
confidential mailing list.
It is why I put the webrevs on the non-public server.
Forgot to switch the server when the test was converted into the jtreg
format.
The public webrevs are:
Hotspot:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/
<http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/>
Jdk:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/
<http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/>
Thanks,
Serguei
-Alan
