RFR: 8220355: Improve assertion texts and exception messages in eventHandlerVMInit

Jean Christophe Beyler jcbeyler at google.com
Fri Mar 15 14:53:10 UTC 2019


Hi Matthias,

(Not an official reviewer :-))

http://cr.openjdk.java.net/~mbaesken/webrevs/8220355.0/:
  - I would invert the test for the environment and do:

if (environment == NULL) {
         abortJVM(jnienv, JPLIS_ERRORMESSAGE_CANNOTSTART ", getting JPLIS
environment failed");
}

and then just un-indent the rest of the code that was in the original if.

http://cr.openjdk.java.net/~mbaesken/webrevs/8220355.0/src/java.instrument/share/native/libinstrument/JPLISAgent.c.udiff.html
  - Nit: the message could consistently start with upper case or lower case
:-)

Apart from that, looks good to me :-)

Thanks,
Jc



On Fri, Mar 15, 2019 at 2:19 AM Baesken, Matthias <matthias.baesken at sap.com>
wrote:

> Hello, please review the following change .
>
>
>
> It enhances  the error  messages in libinstrument in case initialization
> fails .
>
>
>
> ( I found  the enhanced error messages helpful when   analyzing   jtreg
> failures  on one of our platforms in the
> java/lang/instrument/PremainClass/PremainClassTest.java   )
>
>
>
>
>
>
>
> Bug/webrev :
>
>
>
> https://bugs.openjdk.java.net/browse/JDK-8220355
>
>
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8220355.0/
>
>
>
>
>
> Thanks, Matthias
>


-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190315/851fa4ba/attachment.html>


More information about the serviceability-dev mailing list