Re: Jigsaw Enhancement RFR: 8159145 Add JVMTI function GetModuleByPackageName

[email protected] Tue, 21 Jun 2016 23:40:53 -0700

Hi Christian,

I understand that.
Will do my best to achieve this.
There are some corner cases however.


Thanks,
Serguei


On 6/21/16 10:06, Christian Tornqvist wrote:


Hi Serguei,

>Will check if this works well in this case.

>The unique error messages are already printed in the native code.

Yes, they’re printed on stdout/stderr but the test will fail with a“Test failed” exception and our failure matching system wouldn’t beable to tell why it failed. It’d be a lot nicer if it’d just throw anexception when one of the checks in the native code failed.


Thanks,

Christian

*From:*[email protected] [mailto:[email protected]]
*Sent:* Tuesday, June 21, 2016 10:59 AM

*To:* Christian Tornqvist <[email protected]>; 'AlanBateman' <[email protected]>;[email protected]; 'Alexander Kulyakhtin'<[email protected]>; [email protected]*Subject:* Re: Jigsaw Enhancement RFR: 8159145 Add JVMTI functionGetModuleByPackageName


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:*[email protected]
    <mailto:[email protected]>
    [mailto:[email protected]]
    *Sent:* Tuesday, June 21, 2016 6:49 AM
    *To:* Alan Bateman <[email protected]>
    <mailto:[email protected]>;
    [email protected]
    <mailto:[email protected]>; Christian Törnqvist
    <[email protected]>
    <mailto:[email protected]>; Alexander Kulyakhtin
    <[email protected]>
    <mailto:[email protected]>;
    [email protected] <mailto:[email protected]>
    Developers <[email protected]>
    <mailto:[email protected]>
    *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, [email protected]
        <mailto:[email protected]> 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

Re: Jigsaw Enhancement RFR: 8159145 Add JVMTI function GetModuleByPackageName

Reply via email to