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/%7Ejcbeyler/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/>
> >
<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
