Closed Bug 124160 Opened 23 years ago Closed 22 years ago

warning Converting NULL to non-pointer type |jvalue outValue = { NULL };|

Categories

(Core Graveyard :: Java: OJI, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: timeless, Assigned: joshua.xia)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file, 1 obsolete file)

jvalue is a union. http://lxr.mozilla.org/seamonkey/source/embedding/browser/activex/src/pluginhostctrl/pluginsdk_include/jni.h#118

it appears you're trying to clobber the jobject l (pointer to _jobject). Is there a reason not to do jvalue outValue={0}?

4.
modules/oji/src/ProxyJNI.cpp:529 (See build log excerpt)Converting NULL 
to non-pointer type
527 static jvalue InvokeMethod(JNIEnv *env, jobject obj, JNIMethod* 
method, jvalue* args) 528 { 529 jvalue outValue = { NULL }; 530 
ProxyJNIEnv& proxyEnv = GetProxyEnv(env); 531 nsISecureEnv* secureEnv = 
GetSecureEnv(env);5.
modules/oji/src/ProxyJNI.cpp:637 (See build log excerpt)Converting NULL 
to non-pointer type
635 static jvalue InvokeNonVirtualMethod(JNIEnv *env, jobject obj, 
jclass clazz, JNIMethod* method, jvalue* args) 636 { 637 jvalue outValue 
= { NULL }; 638 ProxyJNIEnv& proxyEnv = GetProxyEnv(env); 639 
nsISecureEnv* secureEnv = GetSecureEnv(env);6.
modules/oji/src/ProxyJNI.cpp:741 (See build log excerpt)Converting NULL 
to non-pointer type
739 static jvalue GetField(JNIEnv* env, jobject obj, JNIField* field) 
740 { 741 jvalue outValue = { NULL }; 742 ProxyJNIEnv& proxyEnv = 
GetProxyEnv(env); 743 nsISecureEnv* secureEnv = GetSecureEnv(env);7.
modules/oji/src/ProxyJNI.cpp:824 (See build log excerpt)Converting NULL 
to non-pointer type
822 static jvalue InvokeStaticMethod(JNIEnv *env, jclass clazz, 
JNIMethod* method, jvalue* args) 823 { 824 jvalue outValue = { NULL };
 825 ProxyJNIEnv& proxyEnv = GetProxyEnv(env); 826 nsISecureEnv* 
secureEnv = GetSecureEnv(env);8.
modules/oji/src/ProxyJNI.cpp:928 (See build log excerpt)Converting NULL 
to non-pointer type
926 static jvalue GetStaticField(JNIEnv* env, jclass clazz, JNIField* 
field) 927 { 928 jvalue outValue = { NULL }; 929 ProxyJNIEnv& proxyEnv = 
GetProxyEnv(env); 930 nsISecureEnv* secureEnv = GetSecureEnv(env);
-> me
Assignee: joe.chou → nis
Attached patch Proposed patch (obsolete) — Splinter Review
We can not just use 
  outValue = {0}
because it likely to be the same as 
  outValue.z = 0;
z is the first field of the union and it is the jboolean!
As a result part of the union will be uninitialized.

I proposed to clear double field (because it is longest one).

Please, review.
joe: could you please review proposed change?
I can't remember if OJI uses sr's...
Keywords: patch, review
Back to Joe.
Patch exists but needs to be reviewed.

Assignee: nis → joe.chou
QA Contact: pmac → petersen
reassign to me
Assignee: joe.chou → joshua.xia
Comment on attachment 68533 [details] [diff] [review]
Proposed patch

Joshua,
Can you review this patch?
Attachment #68533 - Flags: review?(joshua.xia)
Comment on attachment 68533 [details] [diff] [review]
Proposed patch

av@netscape.com, beard@netscape.com
Can you review this patch?
Attachment #68533 - Flags: superreview?(beard)
Attachment #68533 - Flags: review?(joshua.xia)
Attachment #68533 - Flags: review?(av)
Comment on attachment 68533 [details] [diff] [review]
Proposed patch

I think the best thing to do would be to define a static const global,

static const jvalue kErrorValue;

and in each method that has a jvalue return type, if the call to the underlying
nsISecureEnv fails, then return that value. Patch coming up.
Attachment #68533 - Flags: superreview?(beard) → superreview-
This uses the return value of the call to the nsISecureEnv to decide whether to
return the local jvalue, or a const error (bottom) value. This removes the need
for initializing the local jvalue altogether.
The original rational for initializing the jvalue to NULL was to ensure that any
jobject (pointer) fields of the jvalue are 0. I don't recall if the bit pattern
of a jdouble (IEEE 64-bit floating point) is all zeroes. If it is, then my
alternative patch should be safe.
Just defining kErrorValue as:

static jvalue kErrorValue;

will result in it being initialized to all zeroes.
IEEE positive zero is all-zeros in 64 bits.

Beard, you wanna use the patch mgr to request r and sr?

/be
Attachment #109135 - Flags: superreview?(brendan)
Attachment #109135 - Flags: review?(joshua.xia)
Attachment #68533 - Attachment is obsolete: true
Attachment #68533 - Flags: superreview-
Attachment #68533 - Flags: review?(av)
Blocks: buildwarning
Comment on attachment 109135 [details] [diff] [review]
Alternative Patch

Seems ok to me, but is this bug well-assigned still?  Timeless, can you find
out?

/be
Attachment #109135 - Flags: superreview?(brendan) → superreview+
Attachment #109135 - Flags: review?(joshua.xia) → review+
Status: NEW → ASSIGNED
checked in per beard
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: