This line was not changed in webrev.01 :)
Anyway looks good.

--alex

On 10/22/2018 12:56, JC Beyler wrote:
Hi Alex,

Done, I left this one:

-            if (!NSK_JVMTI_VERIFY(
-                    jvmti->IterateOverHeap(JVMTI_HEAP_OBJECT_EITHER, heapObjectCallback, (void *)user_data))) {
+            if (!NSK_JVMTI_VERIFY(jvmti->IterateOverHeap(
+                    JVMTI_HEAP_OBJECT_EITHER, heapObjectCallback, (void *)user_data))) {

The whole length was at 126 characters, it seemed a bit long:
http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverHeap/iterheap007/iterheap007.cpp.udiff.html <http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverHeap/iterheap007/iterheap007.cpp.udiff.html>

The new webrev is here:
http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.02/ <http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.02/>

Thanks,
Jc

On Mon, Oct 22, 2018 at 12:44 PM Alex Menkov <alexey.men...@oracle.com <mailto:alexey.men...@oracle.com>> wrote:

    Hi Jc,

    Looks good to me.
    Could you please update
    nsk/jvmti/IterateOverHeap/iterheap007/iterheap007.cpp :

           if (!NSK_JVMTI_VERIFY(
    -            <jvmti_call>)) {
    +            <jvmti_call>)) {

    =>
           if (!NSK_JVMTI_VERIFY(<jvmti_call>)) {

    As all <jvmti_call>s there are quite short.

    --alex


    On 10/19/2018 14:56, JC Beyler wrote:
     > Hi Chris,
     >
     > Done!
     >
     > Here is the newest version:
     > http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.01/
    <http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.01/>
     > <http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.01/>
     >
     > Thanks for the review!
     > Jc
     >
     > On Fri, Oct 19, 2018 at 2:24 PM Chris Plummer
    <chris.plum...@oracle.com <mailto:chris.plum...@oracle.com>
     > <mailto:chris.plum...@oracle.com
    <mailto:chris.plum...@oracle.com>>> wrote:
     >
     >     Hi JC,
     >
     >     iterinstcls006.cpp: Can you fix the indentation of the second
    line.
     >
     >        98             NSK_COMPLAIN2("Local storage was corrupted: %s
     >     ,\n\texpected value: %s\n",
     >        99                              (char *)storage_ptr,
    storage_data);
     >
     >     iterobjreachobj004.cpp: Can you fix the indentation of the
    second line.
     >
     >       123             NSK_COMPLAIN2("Local storage was corrupted: %s
     >     ,\n\texpected value: %s\n",
     >       124                              (char *)storage_ptr,
    storage_data);
     >
     >     iterreachobj002.cpp: You didn't align the arguments like you have
     >     elsewhere.
     >
     >       175
     >     stackReferenceCallbackForSecondObjectsIteration(jvmtiHeapRootKind
     >     root_kind,
     >       176                          jlong     class_tag,
     >       177                          jlong     size,
     >       178                          jlong*    tag_ptr,
     >       179                          jlong     thread_tag,
     >       180                          jint      depth,
     >       181                          jmethodID method,
     >       182                          jint      slot,
     >       183                          void*     user_data) {
     >
     >     iterreachobj004.cpp: Can you fix the indentation of the
    second line.
     >
     >        75         NSK_COMPLAIN2("heapRootCallback: Local storage was
     >     corrupted: %s ,\n\texpected value: %s\n",
     >        76                          (char *)storage_ptr,
    storage_data);
     >
     >       119         NSK_COMPLAIN2("stackReferenceCallback: Local
    storage
     >     was corrupted: %s ,\n\texpected value: %s\n",
     >       120                          (char *)storage_ptr,
    storage_data);
     >
     >       162         NSK_COMPLAIN2("objectReferenceCallback: Local
    storage
     >     was corrupted: %s ,\n\texpected value: %s\n",
     >       163                          (char *)storage_ptr,
    storage_data);
     >
     >     thanks,
     >
     >     Chris
     >
     >     On 10/19/18 1:49 PM, JC Beyler wrote:
     >>     Hi all,
     >>
     >>     Sorry about the spam; forgot to add the subject :)
     >>
     >>     Here is the first of three webrevs to remove spaces around (); I
     >>     also removed any space after !.
     >>
     >>     When the change modified where future parameters should be
     >>     indented, I changed those too (such as
     >>
    
http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html
    
<http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html>
>>  <http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html>)
     >>
     >>     Webrev:
    http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/
    <http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/>
     >>     <http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/>
     >>     Bug: https://bugs.openjdk.java.net/browse/JDK-8212535
     >>
     >>     Thanks!
     >>     Jc
     >>
     >>     On Fri, Oct 19, 2018 at 1:47 PM JC Beyler
    <jcbey...@google.com <mailto:jcbey...@google.com>
     >>     <mailto:jcbey...@google.com <mailto:jcbey...@google.com>>>
    wrote:
     >>
     >>         Hi all,
     >>
     >>         Here is the first of three webrevs to remove spaces
    around ();
     >>         I also removed any space after !.
     >>
     >>         When the change modified where future parameters should be
     >>         indented, I changed those too (such as
     >>
    
http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html
    
<http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html>
>>  <http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html>)
     >>
     >>         Webrev: https://bugs.openjdk.java.net/browse/JDK-8212535
     >>         Bug: https://bugs.openjdk.java.net/browse/JDK-8212535
     >>
     >>         Let me know what you think,
     >>         Jc
     >>
     >>
     >
     >
     >
     > --
     >
     > Thanks,
     > Jc



--

Thanks,
Jc

Reply via email to