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.sriniva...@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.sriniva...@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 >>> >> >> >
