Re spaces :). There’s a bunch of places where “(jvmti_env” should be “(
jvmti_env”, but of course not all of them. I found a bunch. Otherwise, lgtm.,
no need for another webrev.
Paul
In hs104t002.cpp
- if (! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti,
- &caps) )) {
+ if (! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
=>
+ if (! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
In hs203t003.cpp
- if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB4(GetClassSignature,
- jvmti_env, klass, &className, &generic) ) ) {
+ if ( ! NSK_JVMTI_VERIFY (jvmti_env->GetClassSignature(klass, &className,
&generic) ) ) {
=>
+ if ( ! NSK_JVMTI_VERIFY ( jvmti_env->GetClassSignature(klass, &className,
&generic) ) ) {
and
- if (! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti, &caps)
)) {
+ if (! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
=>
+ if (! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
and
- if ( ! NSK_JVMTI_VERIFY( NSK_CPP_STUB3(GetThreadState, jvmti, thread,
&state) ) ) {
+ if ( ! NSK_JVMTI_VERIFY(jvmti->GetThreadState(thread, &state) ) ) {
=>
+ if ( ! NSK_JVMTI_VERIFY( jvmti->GetThreadState(thread, &state) ) ) {
and
- if ( ! NSK_JVMTI_VERIFY( NSK_CPP_STUB3(GetThreadState, jvmti, thread,
&state) ) ) {
+ if ( ! NSK_JVMTI_VERIFY(jvmti->GetThreadState(thread, &state) ) ) {
nsk_printf(" Agent :: Error while getting thread state.\n");
nsk_jvmti_agentFailed();
} else {
if ( state & JVMTI_THREAD_STATE_SUSPENDED) {
- if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2( PopFrame, jvmti, thread)
) ) {
+ if ( ! NSK_JVMTI_VERIFY (jvmti->PopFrame(thread) ) ) {
=>
+ if ( ! NSK_JVMTI_VERIFY( jvmti->GetThreadState(thread, &state) ) ) {
…
+ if ( ! NSK_JVMTI_VERIFY ( jvmti->PopFrame(thread) ) ) {
and
- if ( !NSK_JVMTI_VERIFY( NSK_CPP_STUB2 ( ResumeThread, jvmti, thread)) ) {
+ if ( !NSK_JVMTI_VERIFY(jvmti->ResumeThread(thread)) ) {
=>
+ if ( !NSK_JVMTI_VERIFY( jvmti->ResumeThread(thread)) ) {
In hs203t004.cpp
- if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(GenerateEvents,
jvmti_env,
- JVMTI_EVENT_COMPILED_METHOD_LOAD ) )) {
+ if ( ! NSK_JVMTI_VERIFY
(jvmti_env->GenerateEvents(JVMTI_EVENT_COMPILED_METHOD_LOAD) )) {
=>
+ if ( ! NSK_JVMTI_VERIFY (
jvmti_env->GenerateEvents(JVMTI_EVENT_COMPILED_METHOD_LOAD) )) {
and
- if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB3(GetMethodDeclaringClass,
- jvmti_env, method, &threadClass) ) ) {
+ if ( ! NSK_JVMTI_VERIFY (jvmti_env->GetMethodDeclaringClass(method,
&threadClass) ) ) {
=>
+ if ( ! NSK_JVMTI_VERIFY ( jvmti_env->GetMethodDeclaringClass(method,
&threadClass) ) ) {
and
- if (! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti, &caps)
)) {
+ if (! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
=>
+ if (! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
and
- if ( NSK_JVMTI_VERIFY( NSK_CPP_STUB2(SuspendThread, jvmti, thread) ) ) {
+ if ( NSK_JVMTI_VERIFY(jvmti->SuspendThread(thread) ) ) {
=>
+ if ( NSK_JVMTI_VERIFY( jvmti->SuspendThread(thread) ) ) {
and
- if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB3(GetThreadState, jvmti,
- thread, &state) ) ) {
+ if ( ! NSK_JVMTI_VERIFY (jvmti->GetThreadState(thread, &state) ) ) {
NSK_COMPLAIN0("#error Agent :: while getting thread's state.\n");
nsk_jvmti_agentFailed();
} else {
if ( state & JVMTI_THREAD_STATE_SUSPENDED) {
- if ( ! NSK_JVMTI_VERIFY( NSK_CPP_STUB2(PopFrame, jvmti, thread) )
){
+ if ( ! NSK_JVMTI_VERIFY(jvmti->PopFrame(thread) ) ){
=>
+ if ( ! NSK_JVMTI_VERIFY ( jvmti->GetThreadState(thread, &state) ) ) {
…
+ if ( ! NSK_JVMTI_VERIFY( jvmti->PopFrame(thread) ) ){
In hs204t001.cpp
- /* if( (myTestClass =NSK_CPP_STUB2(NewGlobalRef,jni_env, klass)) ==
NULL) {
+ /* if( (myTestClass =jni_env->NewGlobalRef(klass)) == NULL) {
=>
+ /* if( (myTestClass = jni_env->NewGlobalRef(klass)) == NULL) {
and
- if (!NSK_JNI_VERIFY(env, (testClass =(jclass) NSK_CPP_STUB2(NewGlobalRef,
env, klass)) != NULL))
+ if (!NSK_JNI_VERIFY(env, (testClass =(jclass) env->NewGlobalRef(klass)) !=
NULL))
nsk_jvmti_setFailStatus();
- if (!NSK_JNI_VERIFY(env, (testedThread =NSK_CPP_STUB2(NewGlobalRef, env,
thread)) != NULL))
+ if (!NSK_JNI_VERIFY(env, (testedThread =env->NewGlobalRef(thread)) !=
NULL))
=>
+ if (!NSK_JNI_VERIFY(env, (testClass = (jclass) env->NewGlobalRef(klass))
!= NULL))
nsk_jvmti_setFailStatus();
…
+ if (!NSK_JNI_VERIFY(env, (testedThread = env->NewGlobalRef(thread)) !=
NULL))
and
- if (!NSK_JVMTI_VERIFY( NSK_CPP_STUB2(SuspendThread, jvmti,
thread))) {
+ if (!NSK_JVMTI_VERIFY( jvmti->SuspendThread(thread))) {
=>
+ if (!NSK_JVMTI_VERIFY(jvmti->SuspendThread(thread))) {
In hs204t003.cpp
- if (! NSK_JVMTI_VERIFY( NSK_CPP_STUB3(GetThreadState, jvmti, thread,
&state)) ){
+ if (! NSK_JVMTI_VERIFY(jvmti->GetThreadState(thread, &state)) ){
NSK_DISPLAY0(" Agent :: Error getting thread state.\n");
nsk_jvmti_agentFailed();
} else {
if ( state & JVMTI_THREAD_STATE_SUSPENDED) {
NSK_DISPLAY0(" Agent :: Thread state =
JVMTI_THREAD_STATE_SUSPENDED.\n");
- if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(PopFrame, jvmti, thread) )
) {
+ if ( ! NSK_JVMTI_VERIFY (jvmti->PopFrame(thread) ) ) {
NSK_DISPLAY0("#error Agent :: Jvmti failed to do popFrame.\n");
nsk_jvmti_agentFailed();
} else {
- if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(ResumeThread, jvmti,
thread)) ) {
+ if ( ! NSK_JVMTI_VERIFY (jvmti->ResumeThread(thread)) ) {
=>
+ if (! NSK_JVMTI_VERIFY( jvmti->GetThreadState(thread, &state)) ){
…
+ if ( ! NSK_JVMTI_VERIFY ( jvmti->PopFrame(thread) ) ) {
…
+ if ( ! NSK_JVMTI_VERIFY ( jvmti->ResumeThread(thread)) ) {
In hs301t001.cpp
- if (! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti, &caps)
)) {
+ if (! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
=>
+ if (! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
In hs301t002.cpp
- if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti, &caps)
) ) {
+ if ( ! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) ) ) {
=>
+ if ( ! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) ) ) {
In hs301t003.cpp
- if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB4(GetClassSignature,
- jvmti_env, klass, &className, &generic) ) ) {
+ if ( ! NSK_JVMTI_VERIFY (jvmti_env->GetClassSignature(klass, &className,
&generic) ) ) {
=>
+ if ( ! NSK_JVMTI_VERIFY ( jvmti_env->GetClassSignature(klass, &className,
&generic) ) ) {
and
- if (! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti, &caps)
)) {
+ if (! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
=>
+ if (! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
and
- if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities,
- jvmti, &caps) )) {
+ if ( ! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
=>
+ if ( ! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
From: serviceability-dev <serviceability-dev-boun...@openjdk.java.net> on
behalf of JC Beyler <jcbey...@google.com>
Date: Tuesday, October 16, 2018 at 4:24 PM
To: "alexey.men...@oracle.com" <alexey.men...@oracle.com>
Cc: "serviceability-dev@openjdk.java.net" <serviceability-dev@openjdk.java.net>
Subject: Re: RFR (L) 8211899: Remove the NSK_CPP_STUB macros from vmTestbase
for jvmti/scenarios/[E-M]
Hi all,
How about on a Tuesday? :)
Sneak peak and motivational sentence: after this goes in, we have this
one<http://cr.openjdk.java.net/~jcbeyler/8212148/webrev.00/> which removes the
NSK_CPP_STUBs from the tests entirely! :)
Jc
On Fri, Oct 12, 2018 at 5:37 PM JC Beyler
<jcbey...@google.com<mailto:jcbey...@google.com>> wrote:
Hi all,
Any chance for a second reviewer on a Friday? :)
Jc
On Thu, Oct 11, 2018 at 11:03 AM Alex Menkov
<alexey.men...@oracle.com<mailto:alexey.men...@oracle.com>> wrote:
got it.
LGTM.
--alex
On 10/10/2018 19:03, JC Beyler wrote:
> Hi Alex,
>
> Thanks for the review! Yes I had seen that too before sending this
> review out and forked that fix into this:
> https://bugs.openjdk.java.net/browse/JDK-8211905
>
> Which now is merged and I've updated my webrev to reflect this of course.
>
> My rationale for not doing it here directly is always the same: keeping
> the changes to the "spirit" of what the change is trying to do. Those
> extra casts were a bit out-of-scope and so I just forked the fix in 8211905.
>
> Thanks!
> Jc
>
> On Wed, Oct 10, 2018 at 5:40 PM Alex Menkov
> <alexey.men...@oracle.com<mailto:alexey.men...@oracle.com>
> <mailto:alexey.men...@oracle.com<mailto:alexey.men...@oracle.com>>> wrote:
>
> Hi Jc,
>
> Overall looks good.
>
> one minor note:
>
>
> test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/events/EM06/em06t001/em06t001.cpp:
> - jclassName = (jstring) (jstring) (jstring) (jstring) (jstring)
> (jstring) (jstring) (jstring) (jstring) NSK_CPP_STUB3(CallObjectMethod,
> jni_env, klass,
> - methodID);
> + jclassName = (jstring) (jstring) (jstring) (jstring) (jstring)
> (jstring) (jstring) (jstring) (jstring)
> jni_env->CallObjectMethod(klass,
> methodID);
>
> Please drop multi "(jstring)"
>
> --alex
>
> On 10/08/2018 21:21, JC Beyler wrote:
> > Hi all,
> >
> > I am continuing the NSK_CPP_STUB removal with this next webrev.
> > Webrev: http://cr.openjdk.java.net/~jcbeyler/8211899/webrev.00/
> <http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/>
> > <http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/>
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8211899
> >
> > The change is still straight-forward though, since it is just
> doing the
> > same NSK_CPP_STUB removal. However when I was looking at the
> changes, a
> > lot of these changes are touching lines with spaces before/after
> > parenthesis. I've almost never touched the spaces except if I was
> > refactoring by hand the line at the same time. The rationale
> being that
> > the lines will get fixed a few more times and are, at worse,
> covered by
> > the bug: https://bugs.openjdk.java.net/browse/JDK-8211335, which
> I've
> > commited to doing.
> >
> > Two exceptions are here where I pushed out the code into
> assignments due
> > to really long lines and complex if structures:
> > - jvmti/scenarios/hotswap/HS204/hs204t003/hs204t003.cpp
> >
>
> <http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS204/hs204t003/hs204t003.cpp.udiff.html>
> > - jvmti/scenarios/jni_interception/JI01/ji01t001/ji01t001.cpp
> >
>
> <http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/jni_interception/JI01/ji01t001/ji01t001.cpp.udiff.html>
> >
> > And one exception here where a commented line was doing the
> out-of-if
> > assignment so I just uncommented it and used the variable:
> > - jvmti/scenarios/hotswap/HS301/hs301t001/hs301t001.cpp
> >
>
> <http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS301/hs301t001/hs301t001.cpp.udiff.html>
> >
> > I've tested the modified changes on my machine, all modified
> tests pass.
> >
> > Let me know what you think,
> > Jc
> >
> > Ps: 2 more of these and we can say good bye to NSK_CPP_STUB*
> >
>
>
>
> --
>
> Thanks,
> Jc
--
Thanks,
Jc
--
Thanks,
Jc
