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

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 JNI_OK 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 != JNI_OK || 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
>>>>> JNI_GetCreatedVMs returns successfully!
>>>>>
>>>>
>>>> Ouch!, I will file a CR and fix this.
>>>>
>>> I think HotSpot can only ever return JNI_OK so it may only be a
>>> potential issue when running with another VM.
>>>
>>> -Alan
>>>
>>
>>
>



More information about the core-libs-dev mailing list