RFR (XL) 8211801: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/scenarios/[A-E]

JC Beyler jcbeyler at google.com
Wed Oct 10 18:00:38 UTC 2018


Thanks both for the reviews :)
Jc

On Wed, Oct 10, 2018 at 10:21 AM Alex Menkov <alexey.menkov at oracle.com>
wrote:

> +1
>
> --alex
>
> On 10/08/2018 15:31, serguei.spitsyn at oracle.com wrote:
> > Hi Jc,
> >
> > Thank you for the update!
> > It looks good to me.
> >
> > Thanks,
> > Serguei
> >
> >
> > On 10/8/18 14:53, JC Beyler wrote:
> >> Hi Serguei,
> >>
> >> Normally, I try not to fix issues that already exist before the
> >> change. For the space after the NULL for example, that would get fixed
> >> by https://bugs.openjdk.java.net/browse/JDK-8211335. In this case,
> >> these are the only cases where this happens with a NULL so I fixed them.
> >>
> >> For the space missing before jvmti->X, I fixed my script to handle
> >> this correctly and add them automatically.
> >>
> >> Here is the new webrev:
> >>
> >> Webrev: http://cr.openjdk.java.net/~jcbeyler/8211801/webrev.01
> >> <http://cr.openjdk.java.net/%7Ejcbeyler/8211801/webrev.01>
> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8211801
> >>
> >> Thanks for the review!
> >> Jc
> >>
> >> On Mon, Oct 8, 2018 at 12:46 AM serguei.spitsyn at oracle.com
> >> <mailto:serguei.spitsyn at oracle.com> <serguei.spitsyn at oracle.com
> >> <mailto:serguei.spitsyn at oracle.com>> wrote:
> >>
> >>     Hi Jc,
> >>
> >>
> >>
> http://cr.openjdk.java.net/%7Ejcbeyler/8211801/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP06/ap06t001/ap06t001.cpp.udiff.html
> >>
> >>               if (!NSK_JNI_VERIFY(jni, (fid =
> >>     - NSK_CPP_STUB4(GetStaticFieldID, jni,
> >>     - debugeeClass,
> >>     - "thread",
> >>     - THREAD_CLS_SIGNATURE)) != NULL )) {
> >>     + jni->GetStaticFieldID(debugeeClass, "thread",
> >>     THREAD_CLS_SIGNATURE)) != NULL )) {
> >>                   nsk_jvmti_setFailStatus();
> >>                   break;
> >>               }
> >>
> >>               if (!NSK_JNI_VERIFY(jni, (localRefThread =
> >>     - NSK_CPP_STUB3(GetStaticObjectField, jni,
> >>     - debugeeClass,
> >>     - fid )) != NULL )) {
> >>     + jni->GetStaticObjectField(debugeeClass, fid)) != NULL )) {
> >>                   NSK_COMPLAIN0("GetStaticObjectField returned NULL for
> 'thread' field value\n\n");
> >>                   nsk_jvmti_setFailStatus();
> >>                   break;
> >>               }
> >>
> >>        Could you, please, remove extra space after NULL.
> >>
> >>
> >>     In many-many files (especially, in the foleder:
> scenarios/capability):
> >>
> >>     + if
> >>
>  (!NSK_JVMTI_VERIFY_CODE(JVMTI_ERROR_MUST_POSSESS_CAPABILITY,jvmti->SuspendThread(thread)))
> >>     ... + if
> >>
>  (!NSK_JVMTI_VERIFY_CODE(JVMTI_ERROR_MUST_POSSESS_CAPABILITY,jvmti->ResumeThread(thread)))
> >>     ... + if
> >>
>  (!NSK_JVMTI_VERIFY_CODE(JVMTI_ERROR_MUST_POSSESS_CAPABILITY,jvmti->InterruptThread(thread)))
> >>
> >>
> >>     There are a lot of cases like above where a space before jvmti->
> >>     is missed.
> >>
> >>
> >>     Otherwise, looks good.
> >>
> >>
> >>     Thanks!
> >>     Serguei
> >>
> >>
> >>     On 10/5/18 16:50, JC Beyler wrote:
> >>>     Hi all,
> >>>
> >>>     I continued the NSK_CPP_STUB removal with this next webrev.
> >>>
> >>>     My apologies, this one is big for 50 files; I can further cut it
> >>>     up if we prefer. It is straight-forward though, since it is just
> >>>     doing the same NSK_CPP_STUB removal.
> >>>
> >>>     Without further ado:
> >>>     Webrev: http://cr.openjdk.java.net/~jcbeyler/8211801/webrev.00/
> >>>     <http://cr.openjdk.java.net/%7Ejcbeyler/8211801/webrev.00/>
> >>>     Bug: https://bugs.openjdk.java.net/browse/JDK-8211801
> >>>
> >>>     This does another 50 file batch. I did the same for
> >>>     https://bugs.openjdk.java.net/browse/JDK-8211782 so the scripts
> >>>     can be found there (and added a comment in this bug to link to
> that).
> >>>
> >>>     I've tested the modified changes on my machine, all modified
> >>>     tests pass.
> >>>
> >>>     Let me know what you think,
> >>>     Jc
> >>
> >>
> >>
> >> --
> >>
> >> Thanks,
> >> Jc
> >
>


-- 

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


More information about the serviceability-dev mailing list