On Fri, May 14, 2010 at 4:58 PM, Byeongcheol Lee <lineonk...@gmail.com>wrote:
> Dear Subversion Developers:
>
> My JNI bug detector (Jinn) found several bugs in the JavaHL. I'd like
> to hear your opinion about the bug before filing all the bugs. The bug
> appears the Line 774 in the following slice of source files.
>
> subversion/bindings/javahl/native/CreateJ.cpp
> ....
> 634 jobject
> 635 CreateJ::NotifyInformation(const svn_wc_notify_t *wcNotify)
> 636 {
> ....
> 754 static jmethodID add_mid = 0;
> ....
> 757 add_mid = env->GetMethodID(clazz, "add",
> "(Ljava/lang/Object;)Z");
> ....
> 774 env->CallObjectMethod(jranges, add_mid, jrange);
> ....
>
> The "add" method at Line 757 returns a Java boolean value, but the JNI
> function at Line 774 expects that the "add" method returns a Java
> reference. This usage violates usage rules in JNI specifcation.
>
> "You should replace type in Call<type>Method with the Java type of the
> method you are calling (or use one of the actual method calling
> routine names from the table) and replace NativeType with the
> corresponding native type for that routine."
> [
> http://java.sun.com/javase/6/docs/technotes/guides/jni/spec/functions.html#wp4256
> ]
>
> I propose the following patch:
>
> Index: subversion/bindings/javahl/native/CreateJ.cpp
> ===================================================================
> --- subversion/bindings/javahl/native/CreateJ.cpp (revision 944458)
> +++ subversion/bindings/javahl/native/CreateJ.cpp (working copy)
> @@ -771,7 +771,7 @@
> if (JNIUtil::isJavaExceptionThrown())
> POP_AND_RETURN_NULL;
>
> - env->CallObjectMethod(jranges, add_mid, jrange);
> + env->CallBooleanMethod(jranges, add_mid, jrange);
> if (JNIUtil::isJavaExceptionThrown())
> POP_AND_RETURN_NULL;
>
Byeong,
Thanks for running your tool on our code and reporting the results back to
us. I wrote a log message for the above patch, and committed it in r944525.
For the other instances you mentioned, there isn't a need to create
individual issues. The best way would be to submit the fixes in a series of
logically-grouped patches, following our patch submission guidelines, and
with appropriate log messages.
Thanks again,
-Hyrum
