RFR (XL) 8211801: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/scenarios/[A-E]

[Alex Menkov]

+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
>