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

[Jean Christophe Beyler]

Yes I would fix them all in the next webrev and then continue the porting.
I 100% agree to which is less offensive. I find that

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

to be offensive as well.

So perhaps the TRACED_VARARGS is best:


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

What do you think?


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

> Hi JC,
>
> On 4/29/19 4:25 PM, Jean Christophe Beyler wrote:
>
> 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);
>
> It's not consistent with other uses, or are you suggesting changing them
> all?
>
>
> 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...
>
> I don't like macroizing a method call in this manner (macros should be all
> upper case, right?). Also it's inconsistent with the other tracing calls,
> unless you plan on doing it to all of them.
>
>
> Which looks better?
>
> Not sure. I wouldn't ask which is better. I'd ask which is less offensive.
> :)
>
> thanks,
>
> Chris
>
>
> 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
>
>
>

-- 

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