Pass a pointer to JNI_GetCreatedJavaVMs() instead of null

[David Holmes]

On 7/05/2012 11:27 PM, Alan Bateman wrote:
> On 07/05/2012 11:39, David Holmes wrote:
>>
>> 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!
> I agree that the JNI spec needs clarification and it would be
> inconsistent if this function didn't allow this parameter to be NULL. In
> addition it would be incompatible change (and would likely break
> existing code) if the spec were clarified to disallow NULL.
>
> Another point about this JNI function is that it's not clear (to me
> anyway) how it should behave when there isn't a VM. I think HotSpot
> currently returns JNI_OK and sets *nVMs to 0 (if non-NULL). Assuming the
> JNI spec is clarified to specify existing behavior then passing the
> parameter as NULL just doesn't make sense, meaning JNI_OK does not imply
> that vmBuf[0] is valid.

I think it is clear that if there are no VMs then *nVMs is set to zero 
and JNI_OK is returned. So any code calling this that isn't bothering to 
ask for the number of VMs is assuming there is at least one based on the 
previous actions of the code.

Seems to be a no-win situation from the spec side. Allowing NULL in the 
spec encourages people to not check the value. Disallowing NULL in the 
spec breaks backward compatibility.

With that in mind Kumar's proposed extension to Deven's fix would seem 
the best way forward. Though perhaps with the additional check that nVMs 
== 1.

David
-----

> -Alan.