RFR (M) 8223044: Add back exception checking in tests

[Jean Christophe Beyler]

Hi Serguei,

Thanks for the comments as always.

Originally, I did it this way (where I strive to not change the name of the
variables in the method) so that I changed minimally the lines of the
methods. But since I add a parameter, I change them regardless so I could
go both ways.

Having done this a few times now, I see it right away due to the
TRACE_JNI_CALL added to the call, so there is no confusion for me. But if
you prefer, I can rename all the instances to ec_jni_env. Would you like me
to change in this same webrev all the ones from previous webrevs or just
the ones of this one and I'll do another webrev to make the other ones
consistent?

Thanks,
Jc


On Fri, Apr 26, 2019 at 6:51 PM <serguei.spitsyn at oracle.com> wrote:

> Hi Jc,
>
> It looks good to me.
> A couple of comments/questions.
>
> In most files the changes look similar like below:
>
> -Java_nsk_jvmti_scenarios_bcinstr_BI01_bi01t002_setNewByteCode(JNIEnv *jni_env,+Java_nsk_jvmti_scenarios_bcinstr_BI01_bi01t002_setNewByteCode(JNIEnv *jni,
>                          jobject o, jint ind, jbyteArray byteCode) {
>  +    ExceptionCheckingJniEnvPtr jni_env(jni);
>
>
> But in this file, renaming is in opposite direction:
>
> -agentProc(jvmtiEnv* jvmti, JNIEnv* jni, void* arg) {+agentProc(jvmtiEnv* jvmti, JNIEnv* jni_env, void* arg) {
>  +    ExceptionCheckingJniEnvPtr jni(jni_env);
>
>
> In general, I have a little concern about use of jni_env for ExceptionCheckingJniEnvPtr.
> Could it be less confusing to use something like ec_jni or ec_jni_env?
> So that it will be clear that it is not normal (JNIEnv *).
>
> Thanks,
> Serguei
>
>
> On 4/26/19 5:52 PM, Jean Christophe Beyler wrote:
>
> Hi all,
>
> (Re-sending with subject line complete :-))
>
> Since JDK-8213501 finally merged (sorry it took so long), I am able to
> continue this work. Here is the work that puts back the messages for any
> calls that were moved around due to JDK-8212884.
>
> Webrev: http://cr.openjdk.java.net/~jcbeyler/8223044/webrev.00/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8223044
>
> All modified tests pass on my local dev machine.
>
> Thanks,
> Jc
>
>
>

-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190426/257ebfed/attachment-0001.html>