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

Jean Christophe Beyler jcbeyler at google.com
Mon Apr 29 23:25:16 UTC 2019


Hi Chris,

I agree it's not ideal, how about putting it first?

  73     loadedClass = (jclass)
jni_env->CallObjectMethod(TRACE_JNI_CALL, loader, methodID, className);

I find that less awkward than the VAR_ARGS, no?

Or something like:
 73     loadedClass = (jclass) jni_env->CallObjectMethodTraced(loader,
methodID, className);

Where CallObjectMethodTraced is actually a define in the exception handling
system that adds the line and filename...

Which looks better?

Thanks again,
Jc

On Mon, Apr 29, 2019 at 3:40 PM Chris Plummer <chris.plummer at oracle.com>
wrote:

> Ok. I only half heatedly suggest the following
>
>   73     loadedClass = (jclass) jni_env->CallObjectMethod(loader,
> methodID, VAR_ARGS(className));
>
> And then VAR_ARGS can contain the reference to TRACE_JNI_CALL.
>
> Chris
>
> On 4/29/19 2:58 PM, Jean Christophe Beyler wrote:
>
> Hi Chris,
>
> Thanks for the review! It is the only issue I have with the system now and
> I had not found a good solution for:
>
> as CallObjectMethod is a variadic method, I can't put TRACE_JNI_CALL at
> the end; so I put right before the end; that allows me to do this:
>
>
> +jobject ExceptionCheckingJniEnv::CallObjectMethod(jobject obj, jmethodID methodID, int line,+                         const char* file_name, ...) {+  JNIVerifier<> marker(this, "CallObjectMethod", obj, methodID, line, file_name);++  va_list args;+  va_start(args, file_name);+  jobject result = _jni_env->CallObjectMethodV(obj, methodID, args);+  va_end(args);+  return result;+}
>
> Putting it at the end would mean I would have to play with the va_list to remove the last one, and from what I have understood with va_list, it's kind of a no-no to do so. So I left at this.
>
> Do you have any better suggestion?
>
> Thanks!
>
> Jc
>
>
> On Mon, Apr 29, 2019 at 10:38 AM Chris Plummer <chris.plummer at oracle.com>
> wrote:
>
>> Hi JC,
>>
>> In em01t002.cpp, is this correct?
>>
>>   73     loadedClass = (jclass) jni_env->CallObjectMethod(loader,
>> methodID, TRACE_JNI_CALL, className);
>>
>> Shouldn't the TRACE_JNI_CALL arg be last? If so, can you look into why
>> this test didn't fail as a result.
>>
>> Other than that the changes look good.
>>
>> thanks,
>>
>> Chris
>>
>> 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
>
>
>

-- 

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


More information about the serviceability-dev mailing list