RFR (M) 8213003: Remove more assignments in ifs for vmTestbase/[a-s]

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Wed Oct 31 18:30:18 UTC 2018

It prints the FILE/LINE which is enough to identify the error point.
But I agree that doing the assignments within the NSK_JNI_VERIFY was 
intentional as it gives more context.
 From the other hand it make the code ugly and less readable.
Not sure, what direction is better to go.


On 10/31/18 11:21, Chris Plummer wrote:
> Hi Claes,
> It's good that you brought this up. NSK_JNI_VERIFY is always "on", but 
> moving the assignment outside of it does slightly change the behavior 
> of the macro (although the code still executes "correctly"):
> /**
>  * Execute action with JNI call, check result for true and
>  * pending exception and complain error if required.
>  * Also trace action execution if tracing mode enabled.
>  */
> #define NSK_JNI_VERIFY(jni, action)  \
> (nsk_ltrace(NSK_TRACE_BEFORE,__FILE__,__LINE__,"%s\n",#action), \
> nsk_jni_lverify(NSK_TRUE,jni,action,__FILE__,__LINE__,"%s\n",#action))
> So you will no longer see the JNI call in the trace or error output. 
> You will just see the comparison to the JNI result, which gives no 
> context as to the source of the result. So I'm now starting to think 
> that doing the assignments within the NSK_JNI_VERIFY was intentional 
> so we would see the JNI call in the trace output.
> Chris
> On 10/31/18 3:16 AM, Claes Redestad wrote:
>> Hi,
>> here's a case that your script seems to miss:
>> http://cr.openjdk.java.net/%7Ejcbeyler/8213003/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassLoaderClasses/clsldrclss002/clsldrclss002.cpp.udiff.html 
>>      if (!NSK_JNI_VERIFY(jni, (testedFieldID =
>>              jni->GetStaticFieldID(testedClass, FIELD_NAME, 
>> I'd note that with some of your changes here you're unconditionally 
>> executing logic outside of NSK_JNI_VERIFY macros. Does care need be 
>> taken to wrap the code blocks in equivalent macros/ifdefs? Or are the 
>> NSK_JNI_VERIFY macros always-on in these tests (and essentially 
>> pointless)?
>> /Claes
>> On 2018-10-31 05:42, JC Beyler wrote:
>>> Hi all,
>>> I worked on updating my script and handling more assignments so I 
>>> redid a second pass on the same files to get all the cases. Now, on 
>>> those files the regex "if.* = " no longer shows any cases we would 
>>> want to fix.
>>> Could I get a review for this webrev:
>>> Webrev: http://cr.openjdk.java.net/~jcbeyler/8213003/webrev.00/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8213003
>>> I tested this on my dev machine by passing all the tests that were 
>>> modified.
>>> Thanks!
>>> Jc

More information about the serviceability-dev mailing list