RFR (M) 8212535: Remove spaces before/after () for vmTestbase/[a-s]*

Hohensee, Paul hohensee at amazon.com
Tue Oct 23 00:23:30 UTC 2018 

Lgtm. One nit in libnativeGC01.cpp
- * A C function that takes a reference to java Object( a circular Linked list)
+ * A C function that takes a reference to java Object(a circular Linked list)

=>


+ * A C function that takes a reference to java Object (a circular Linked list)

No need for a new webrev.

Paul

From: serviceability-dev <serviceability-dev-bounces at openjdk.java.net> on behalf of "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com>
Date: Monday, October 22, 2018 at 2:40 PM
To: JC Beyler <jcbeyler at google.com>, "alexey.menkov at oracle.com" <alexey.menkov at oracle.com>
Cc: "serviceability-dev at openjdk.java.net" <serviceability-dev at openjdk.java.net>
Subject: Re: RFR (M) 8212535: Remove spaces before/after () for vmTestbase/[a-s]*

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:
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.menkov at oracle.com<mailto:alexey.menkov at 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.plummer at oracle.com<mailto:chris.plummer at oracle.com>
> <mailto:chris.plummer at oracle.com<mailto:chris.plummer at 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 <jcbeyler at google.com<mailto:jcbeyler at google.com>
>>     <mailto:jcbeyler at google.com<mailto:jcbeyler at 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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181023/4ef069bf/attachment-0001.html>

More information about the serviceability-dev mailing list