Pass a pointer to JNI_GetCreatedJavaVMs() instead of null / review please (original) (raw)

Vitaly Davidovich vitalyd at gmail.com
Mon May 7 21:52:47 UTC 2012

I agree it doesn't really matter; using JNI_OK is arguably slightly better as it (1) doesn't assume anything about what non negative value it'll assume and (2) uses the constant defined specifically for this, but I agree it's insignificant in the grand scheme of things.

Cheers

Sent from my phone On May 7, 2012 5:34 PM, "Kumar Srinivasan" <kumar.x.srinivasan at oracle.com> wrote:

Hi Vitaly,

The JNI Spec says the following: "Returns JNIOK on success; returns a suitable JNI error code (a negative number) on failure." It doesn't really matter, if others feel strongly about it, I will change it. Kumar

Hi Kumar, Based on the discussion, should it check for a (retval != JNIOK || vm == null) instead of (retval < 0 || vm == null)?_ _Regards,_ _Vitaly_ _Sent from my phone_ _On May 7, 2012 4:23 PM, "Kumar Srinivasan" <kumar.x.srinivasan at oracle.com> wrote: Hi,

Please review http://cr.openjdk.java.net/~ksrini/7166955

Thanks Kumar On 07/05/2012 16:45, Kumar Srinivasan wrote:

Hi David, Deven, Alan,

The spec doesn't say anything but the implementation does check for NULL. I think this is a spec issue rather than a code issue (and I think hotspot-runtime owns the JNI spec so cc'ed). It is common practice for API's that take pointers like this to say "if buf is not NULL then the value of XXX is written into buf". Particularly as in this case there will only ever be at most 1 VM created per-process anyway.

I'm more concerned about the fact that the code doesn't even check if JNIGetCreatedVMs returns successfully!

Ouch!, I will file a CR and fix this. I think HotSpot can only ever return JNIOK so it may only be a potential issue when running with another VM. -Alan

More information about the core-libs-dev mailing list