RFR (XL) 8210385: Clean up JNI_ENV_ARG and factorize the macros for remaining vmTestbase/jvmti tests

[JC Beyler]

Hi Alex,

Done! Anyone else motivated in looking at part 1 out of 4 to remove all
JNI_ENV* from our vmTestbase tests?

Webrev: http://cr.openjdk.java.net/~jcbeyler/8210385/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8210385

Thanks!
Jc

On Fri, Sep 7, 2018 at 2:07 PM Alex Menkov <alexey.menkov at oracle.com> wrote:

> Hi Jc,
>
> The fix looks good.
> The only note:
>
>
> test/hotspot/jtreg/vmTestbase/nsk/jvmti/IsFieldSynthetic/isfldsin003/isfldsin003.cpp
>
> contains double jvmti initialization:
> -    res = JNI_ENV_PTR(jvm)->GetEnv(JNI_ENV_ARG(jvm, (void **) &jvmti),
> -        JVMTI_VERSION_1_1);
> +    res = jvm->GetEnv((void **) &jvmti, JVMTI_VERSION_1_1);
>       if (res != JNI_OK || jvmti == NULL) {
>           printf("Wrong result of a valid call to GetEnv!\n");
>           return JNI_ERR;
>       }
>
> -    res = JNI_ENV_PTR(jvm)->GetEnv(JNI_ENV_ARG(jvm, (void **) &jvmti),
> -        JVMTI_VERSION_1_1);
> +    res = jvm->GetEnv((void **) &jvmti, JVMTI_VERSION_1_1);
>       if (res != JNI_OK || jvmti == NULL) {
>           printf("Wrong result of a valid call to GetEnv!\n");
>           return JNI_ERR;
>       }
> Could you please remove one of them.
>
> --alex
>
> On 09/07/2018 12:22, JC Beyler wrote:
> > Hi Alex,
> >
> > Sounds good to me, I'll aim between 40~50 files then :)
> >
> > Here is the jvmti/A-N* remaining refactoring of the JNI_ENV* /
> > JVMTI_ENV* macros:
> >
> > I have updated the bug to reflect this:
> >
> > Webrev: http://cr.openjdk.java.net/~jcbeyler/8210385/webrev.01/
> > <http://cr.openjdk.java.net/%7Ejcbeyler/8210385/webrev.01/>
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8210385
> >
> > Thanks again!
> > Jc
> >
> >
> > On Fri, Sep 7, 2018 at 11:51 AM Alex Menkov <alexey.menkov at oracle.com
> > <mailto:alexey.menkov at oracle.com>> wrote:
> >
> >     I think ~40-50 files per fix is my maximum :)
> >
> >     --alex
> >
> >     On 09/07/2018 11:37, JC Beyler wrote:
> >      > Sounds good to me, would you prefer it in 4 webrevs of 1k
> >     changes? Or 2
> >      > webrevs of 2k changes?
> >      >
> >      > I can do either easily, so just let me know; your job as the
> >     reviewer is
> >      > harder for these :)
> >      > Jc
> >      >
> >      > On Fri, Sep 7, 2018 at 11:34 AM Alex Menkov
> >     <alexey.menkov at oracle.com <mailto:alexey.menkov at oracle.com>
> >      > <mailto:alexey.menkov at oracle.com
> >     <mailto:alexey.menkov at oracle.com>>> wrote:
> >      >
> >      >     Hi Jc,
> >      >
> >      >     I'd prefer to see the fix splitted to smaller parts.
> >      >     The changes are quite simple, but it's just too many files,
> >     so it's
> >      >     impossible (at least for me) to review them accurately.
> >      >
> >      >     --alex
> >      >
> >      >     On 09/07/2018 10:53, JC Beyler wrote:
> >      >      > Hi all,
> >      >      >
> >      >      > I'm taking the risk to perhaps do something too huge but
> >     as it is
> >      >     the
> >      >      > same as the previous ones (and the last one!):
> >      >      >
> >      >      > It's 4k of changes, for that I apologize. I can divide it
> >     up in 2/3
> >      >      > parts again if you want or we can just do it in one go
> >     since now
> >      >     some
> >      >      > reviewers know what to expect. There still is a bit of
> >      >     refactoring to
> >      >      > the vmTestbase but this will in essence finish:
> >      >      >    - Removing the JNI_ENV* and JVMTI_ENV* macros for the
> >     vmTestbase
> >      >      >
> >      >      > Webrev:
> >     http://cr.openjdk.java.net/~jcbeyler/8210385/webrev.00/
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8210385/webrev.00/>
> >      >     <http://cr.openjdk.java.net/%7Ejcbeyler/8210385/webrev.00/>
> >      >      > <http://cr.openjdk.java.net/%7Ejcbeyler/8210385/webrev.00/
> >
> >      >      > Bug: https://bugs.openjdk.java.net/browse/JDK-8210385
> >      >      >
> >      >      > Thanks for your reviews and let me know if you would
> >     prefer me to
> >      >     cut it
> >      >      > up like I did the Get[A-F] & Get[G-Z] ones,
> >      >      > Jc
> >      >      >
> >      >      > Ps: for the curious, then we can start cleaning up the
> native
> >      >     code a bit
> >      >      > more sanely (example: JDK-8191519) and there will be one
> >     more webrev
> >      >      > removing #ifdef cplusplus lingering in the vmTestbase
> >     (JDK-8210481).
> >      >      >
> >      >
> >      >
> >      >
> >      > --
> >      >
> >      > Thanks,
> >      > Jc
> >
> >
> >
> > --
> >
> > Thanks,
> > Jc
>


-- 

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