|
Looks good.
Chris
On 8/1/19 6:55 PM, Jean Christophe
Beyler wrote:
Hi Serguei,
Thanks :)
Done, I updated it and the copyrights and did an in place
replacement:
Thanks again,
Jc
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:
Thanks for your review :)
Jc
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
Udiffs
Sdiffs
Frames
Old
New
----- Raw
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 (! .*).
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
--
--
|
