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.hol...@oracle.com
<mailto: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>
> <mailto: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
