|
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.
Not sure. I wouldn't ask which is better. I'd ask which is less
offensive. :)
thanks,
Chris
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
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.
All modified tests pass on my local dev
machine.
--
--
|
