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