RFR (XS) 8215495: Always set isCopy

[yumin qi]

Looks good to me.

Thanks
Yumin

On Wed, Dec 19, 2018 at 7:42 AM JC Beyler <jcbeyler at google.com> wrote:

> 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/18133be5/attachment-0001.html>