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:

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

Reply via email to