Hi Serguei, Thanks :)
Done, I updated it and the copyrights and did an in place replacement: Webrev: http://cr.openjdk.java.net/~jcbeyler/8228998/webrev.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8228998 Thanks again, Jc On Thu, Aug 1, 2019 at 5:27 PM <serguei.spit...@oracle.com> wrote: > 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/ > Bug: https://bugs.openjdk.java.net/browse/JDK-8228998 > > Thanks for your review :) > Jc > > > On Thu, Aug 1, 2019 at 4:07 PM <serguei.spit...@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/ >> 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 > > > -- Thanks, Jc
