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

[Baesken, Matthias]

Hi JC, thanks for your comments .
Second webrev , may I have a second review please ?

http://cr.openjdk.java.net/~mbaesken/webrevs/8220355.1/

One question that came to my mind :  do we still have to worry about the old-style C  variable declaration order  in eventHandlerVMInit   after  changing the test to
  if (environment == NULL) {   … }
or is this resolved now on all platforms ?


Best regards, Matthias


From: Jean Christophe Beyler <jcbeyler at google.com>
Sent: Freitag, 15. März 2019 15:53
To: Baesken, Matthias <matthias.baesken at sap.com>
Cc: serviceability-dev at openjdk.java.net
Subject: Re: RFR: 8220355: Improve assertion texts and exception messages in eventHandlerVMInit

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<mailto: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/20190318/1e0074aa/attachment.html>