Sounds good to me: For the odd corner case:
Webrev: http://cr.openjdk.java.net/~jcbeyler/8215495/webrev.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215495 Thanks! Jc On Mon, Dec 17, 2018 at 2:39 PM David Holmes <david.hol...@oracle.com> wrote: > Hi Jc, > > On 18/12/2018 8:12 am, JC Beyler wrote: > > Hi David, > > > > I understand the rationale behind the "If the method does return NULL, > > then isCopy's value is undefined". But what about > > the DEFINE_GETSCALARARRAYELEMENTS case? > > > > It does this: > > if (len == 0) { \ > > /* Empty array: legal but useless, can't return NULL. \ > > * Return a pointer to something useless. \ > > * Avoid asserts in typeArrayOop. */ \ > > result = (ElementType*)get_bad_address(); \ > > > > Should we at least then put isCopy to JNI_FALSE in that case since it > > does not return NULL and no exception is raised, > > Sure - it's an odd corner case, but better not to leave isCopy > potentially uninitialized. > > Thanks, > David > > > Thanks, > > Jc > > > > > > On Mon, Dec 17, 2018 at 1:29 PM David Holmes <david.hol...@oracle.com > > <mailto:david.hol...@oracle.com>> wrote: > > > > Hi Jc, > > > > On 18/12/2018 3:42 am, JC Beyler wrote: > > > Hi all, > > > > > > Could I get a review for this webrev: > > > > > > Webrev: http://cr.openjdk.java.net/~jcbeyler/8215495/webrev.00/ > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8215495 > > > > isCopy only has to be set if the method executes successfully > > > > "If isCopy is not NULL, then *isCopy is set to JNI_TRUE if a copy is > > made; or it is set to JNI_FALSE if no copy is made." > > > > You can only make (or not) a copy if the operation actually > > succeeds. So > > before checking isCopy the caller must check for NULL and/or a > pending > > exception. > > > > I see no bug here. > > > > David > > ----- > > > > > Thanks, > > > Jc > > > > > > > > -- > > > > Thanks, > > Jc > -- Thanks, Jc
