Why should there be a space after the '(' ? That's not hotspot-style.
Neither is a space before ')'.
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) )) {
*Date: *Tuesday, October 16, 2018 at 4:24 PM
*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/%7Ejcbeyler/8212148/webrev.00/> which
removes the NSK_CPP_STUBs from the tests entirely! :)
Jc
Hi all,
Any chance for a second reviewer on a Friday? :)
Jc
On Thu, Oct 11, 2018 at 11:03 AM Alex Menkov
got it.
LGTM.
--alex
Post by JC BeylerHi Alex,
Thanks for the review! Yes I had seen that too before sending
this
Post by JC Beylerhttps://bugs.openjdk.java.net/browse/JDK-8211905
Which now is merged and I've updated my webrev to reflect
this of course.
Post by JC BeylerMy rationale for not doing it here directly is always the
same: keeping
Post by JC Beylerthe changes to the "spirit" of what the change is trying to
do. Those
Post by JC Beylerextra casts were a bit out-of-scope and so I just forked the
fix in 8211905.
Post by JC BeylerThanks!
Jc
On Wed, Oct 10, 2018 at 5:40 PM Alex Menkov
Hi Jc,
Overall looks good.
- jclassName = (jstring) (jstring) (jstring) (jstring)
(jstring)
Post by JC Beyler (jstring) (jstring) (jstring) (jstring)
NSK_CPP_STUB3(CallObjectMethod,
Post by JC Beyler jni_env, klass,
- methodID);
+ jclassName = (jstring) (jstring) (jstring) (jstring)
(jstring)
Post by JC Beyler (jstring) (jstring) (jstring) (jstring)
jni_env->CallObjectMethod(klass,
methodID);
Please drop multi "(jstring)"
--alex
> Hi all,
>
> I am continuing the NSK_CPP_STUB removal with this
next webrev.
http://cr.openjdk.java.net/~jcbeyler/8211899/webrev.00/
<http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/>
Post by JC Beyler <http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/>
>
<http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/>
Post by JC Beyler > Bug: https://bugs.openjdk.java.net/browse/JDK-8211899
>
> The change is still straight-forward though, since it
is just
Post by JC Beyler doing the
> same NSK_CPP_STUB removal. However when I was looking
at the
Post by JC Beyler changes, a
> lot of these changes are touching lines with spaces
before/after
Post by JC Beyler > parenthesis. I've almost never touched the spaces
except if I was
Post by JC Beyler > refactoring by hand the line at the same time. The
rationale
Post by JC Beyler being that
> the lines will get fixed a few more times and are, at
worse,
https://bugs.openjdk.java.net/browse/JDK-8211335, which
Post by JC Beyler I've
> commited to doing.
>
> Two exceptions are here where I pushed out the code into
assignments due
> - 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>
Post by JC Beyler >
> And one exception here where a commented line was
doing the
Post by JC Beyler out-of-if
> - 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>
Post by JC Beyler >
> I've tested the modified changes on my machine, all
modified
Post by JC Beyler 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
--