RFR (XS) 8215495: Always set isCopy

[JC Beyler]

Hi all,

Could I get a second review for this please? :)

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 4:25 PM David Holmes <david.holmes at oracle.com>
wrote:

> LGTM.
>
> Thanks,
> David
>
> On 18/12/2018 8:53 am, JC Beyler wrote:
> > Sounds good to me:
> >
> > For the odd corner case:
> >
> > Webrev: http://cr.openjdk.java.net/~jcbeyler/8215495/webrev.01/
> > <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.holmes at oracle.com
> > <mailto:david.holmes at 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.holmes at oracle.com <mailto:david.holmes at oracle.com>
> >      > <mailto:david.holmes at oracle.com
> >     <mailto:david.holmes at 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
>


-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181219/324d89e2/attachment.html>