Hi Alex,

It looks good.

Thanks!
Serguei


On 10/22/18 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:

The new webrev is here:

Thanks,
Jc

On Mon, Oct 22, 2018 at 12:44 PM Alex Menkov <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/>
>
> 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>> 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>)
>>
>>     Webrev: http://cr.openjdk.java.net/~jcbeyler/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>> 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>)
>>
>>         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