In the warnings logs on Linux tinderbox builds, I see these (potentially?) serious bugs: 3. js/src/liveconnect/jsj.c:681 (See build log excerpt) `jEnv' might be used uninitialized in this function 679 JSJ_AttachCurrentThreadToJava(JSJavaVM *jsjava_vm, const char *name, JNIEnv **java_envp) 680 { 681 JNIEnv *jEnv; 682 SystemJavaVM *java_vm; 683 JSJavaThreadState *jsj_env; 4. js/src/liveconnect/jsj.c:730 (See build log excerpt) `java_vm' might be used uninitialized in this function 728 { 729 JSJavaThreadState *jsj_env; 730 SystemJavaVM *java_vm; 731 JSJavaVM *jsjava_vm;

to default

Reassigning to Patrick -

Currently TBox shows 3 warnings in Live Connect: js/src/liveconnect/jsj.c:698 `jEnv' might be used uninitialized in this function js/src/liveconnect/jsj.c:753 `java_vm' might be used uninitialized in this function js/src/liveconnect/jsj_JSObject.c:253 `java_wrapper_obj' might be used uninitialized in this function P.S. trying to make 1.0 as warnings-free as possible...

This patch fixes two warnings: jsj.c: In function `JSJ_AttachCurrentThreadToJava': jsj.c:680: warning: `jEnv' might be used uninitialized in this function jsj.c: In function `jsj_MapJavaThreadToJSJavaThreadState': jsj.c:735: warning: `java_vm' might be used uninitialized in this function In both cases the code structure is about the same: ... * var; ... if (condition) var = ...; if (var == NULL) rerutn NULL; Of course, if "condition" happens to be false, we'll end up comparing an uninitialized portion of memory to NULL with un[predictable results! The patch preinitializes variables to NULL and gets rid of uncertanty.

Comment on attachment 102553 [details] [diff] [review] Fix two warnings (that also appear to be bugs!) r=brade

Can I get an sr for the patch? (And can somebody check it in?) Thanks!

Comment on attachment 102553 [details] [diff] [review] Fix two warnings (that also appear to be bugs!) Yow. How'd that happen? sr=brendan@mozilla.org, please mail drivers for 1.2 branch permission. Does this need to be fixed on the 1.0 branch also? /be

> please mail drivers for 1.2 branch permission Can somebody with CVS permissions take care of this? > Does this need to be fixed on the 1.0 branch also? The warnings do exist on 1.0 branch. As far as "need to be fixed" goes, I do not know if they ever cause anything bad to happen...

This problem is largely academic, since the only way jEnv ends up being unitialized is if (JSJ_callbacks && JSJ_callbacks->attach_current_thread) is FALSE. A clearer change might instead be: Index: mozilla/js/src/liveconnect/jsj.c =================================================================== RCS file: /cvsroot/mozilla/js/src/liveconnect/jsj.c,v retrieving revision 1.27 diff -b -u -2 -r1.27 jsj.c --- mozilla/js/src/liveconnect/jsj.c 1 Aug 2002 11:04:18 -0000 1.27 +++ mozilla/js/src/liveconnect/jsj.c 7 Nov 2002 23:54:26 -0000 @@ -688,6 +692,6 @@ /* Try to attach a Java thread to the current native thread */ java_vm = jsjava_vm->java_vm; - if (JSJ_callbacks && JSJ_callbacks->attach_current_thread) - jEnv = JSJ_callbacks->attach_current_thread(java_vm); + jEnv = (JSJ_callbacks && JSJ_callbacks->attach_current_thread) ? + JSJ_callbacks->attach_current_thread(java_vm) : NULL; if (jEnv == NULL) return NULL; @@ -745,6 +749,6 @@ /* First, figure out which Java VM is calling us */ - if (JSJ_callbacks && JSJ_callbacks->get_java_vm) - java_vm = JSJ_callbacks->get_java_vm(jEnv); + java_vm = (JSJ_callbacks && JSJ_callbacks->get_java_vm) ? + JSJ_callbacks->get_java_vm(jEnv) : NULL; if (java_vm == NULL) return NULL;

I'd be happy to get this patch checked in.

Created a cleaner version of the fix, similar to the one requested in comment #9.

Comment on attachment 111677 [details] [diff] [review] Fix two warnings (that also appear to be bugs), v2 Can you check this in? Thanks!

Comment on attachment 111677 [details] [diff] [review] Fix two warnings (that also appear to be bugs), v2 r=beard

Checked into trunk.

Per comment #14.

TBox shows the warnings are gone.