RFR (L) 8228998: Remove the testing against NSK_FALSE from tests

[serguei.spitsyn at oracle.com]

Hi Jc,

Looks good.

This still aligned incorrectly:

221 static bool getFieldsAndObjects(jvmtiEnv* jvmti,
222 JNIEnv* jni,
223 jclass debugeeClass,
224 jclass rootObjectClass,
225 jclass chainObjectClass,
226 jobject* rootObjectPtr,
227 jfieldID* reachableChainField,
228 jfieldID* unreachableChainField,
229 jfieldID* nextField) {


Some copyright comments need an update.
No need in another review if you fix it.
You may want to update the webrev in place for other reviewers.

Thanks,
Serguei


On 8/1/19 4:53 PM, Jean Christophe Beyler wrote:
> Hi Serguei,
>
> My apologies. I fixed the forbidden on the old webrev link. Then I 
> rechecked the white-spaces, and made webrev not ignore white space 
> changes. Here is the new webrev:
>
> Webrev: http://cr.openjdk.java.net/~jcbeyler/8228998/webrev.01/ 
> <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev.01/>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8228998
>
> Thanks for your review :)
> Jc
>
>
> On Thu, Aug 1, 2019 at 4:07 PM <serguei.spitsyn at oracle.com 
> <mailto:serguei.spitsyn at oracle.com>> wrote:
>
>     Hi Jc,
>
>     Thank you for taking care about this!
>     Most of the links in the webrev can not be resolved.
>     I'm getting the error: "403 - Forbidden".
>
>     The only item that works is:
>
>     |Cdiffs
>     <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.cdiff.html>
>     Udiffs
>     <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.udiff.html>
>     Sdiffs
>     <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.sdiff.html>
>     Frames
>     <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.frames.html>
>     Old
>     <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp-.html>
>     New
>     <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.html>
>     ----- Raw
>     <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/raw_files/new/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp>
>
>     |
>     *test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp*
>
>
>     Also, the patch is readable:
>     http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/jdk-false.changeset
>
>
>     It looks pretty good.
>
>     Only a one comments:
>
>     http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.frames.html
>
>     A couple of fragments are not aligned properly:
>
>     221 static bool getFieldsAndObjects(jvmtiEnv* jvmti,
>       222                                 JNIEnv*     jni,
>       223                                 jclass      debugeeClass,
>       . . .
>
>     434 static bool checkTestedObjects(jvmtiEnv* jvmti,
>       435                               JNIEnv*    jni,
>       436                               int        chainLength,
>       437                               ObjectDesc objectDescList[])
>
>
>     Thanks,
>     Serguei
>
>
>     On 8/1/19 3:16 PM, Jean Christophe Beyler wrote:
>>     Hi all,
>>
>>     It took me a while to pick this item back but here we go :-).
>>     Here is a webrev that removes all the if (.* == NSK_FALSE) and
>>     replaces them with if (! .*).
>>
>>     Webrev: http://cr.openjdk.java.net/~jcbeyler/8228998/webrev/
>>     <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/>
>>     Bug: https://bugs.openjdk.java.net/browse/JDK-8228998
>>
>>     For the EM tests, I also updated the returns to be boolean,
>>     entirely removing the NSK_FALSE/NSK_TRUE parts because of the way
>>     the tests were done. Let me know if you'd rather I divide those up.
>>
>>     This was tested by running the tests changed on my dev machine,
>>     I'll push it to the submit repo after review :-)
>>
>>     Thanks and have a great evening,
>>     Jc
>
>
>
> -- 
>
> Thanks,
> Jc

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190801/6d57e0be/attachment-0001.html>