Discussion:
RFR (M) 8212535: Remove spaces before/after () for vmTestbase/[a-s]*
(too old to reply)
JC Beyler
2018-10-19 20:47:28 UTC
Permalink
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
)

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
JC Beyler
2018-10-19 20:49:22 UTC
Permalink
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
)

Webrev: http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8212535

Thanks!
Jc
Post by JC Beyler
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
)
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
Chris Plummer
2018-10-19 21:24:25 UTC
Permalink
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi JC,<br>
<br>
iterinstcls006.cpp: Can you fix the indentation of the second
line.<br>
<br>
  98             NSK_COMPLAIN2("Local storage was corrupted: %s
,\n\texpected value: %s\n",<br>
  99                              (char *)storage_ptr,
storage_data);<br>
<br>
iterobjreachobj004.cpp: Can you fix the indentation of the second
line.<br>
<br>
 123             NSK_COMPLAIN2("Local storage was corrupted: %s
,\n\texpected value: %s\n",<br>
 124                              (char *)storage_ptr,
storage_data);<br>
<br>
iterreachobj002.cpp: You didn't align the arguments like you have
elsewhere.<br>
<br>
 175
stackReferenceCallbackForSecondObjectsIteration(jvmtiHeapRootKind
root_kind,<br>
 176                          jlong     class_tag,<br>
 177                          jlong     size,<br>
 178                          jlong*    tag_ptr,<br>
 179                          jlong     thread_tag,<br>
 180                          jint      depth,<br>
 181                          jmethodID method,<br>
 182                          jint      slot,<br>
 183                          void*     user_data) {<br>
<br>
iterreachobj004.cpp: Can you fix the indentation of the second
line.<br>
<br>
  75         NSK_COMPLAIN2("heapRootCallback: Local storage was
corrupted: %s ,\n\texpected value: %s\n",<br>
  76                          (char *)storage_ptr, storage_data);<br>
<br>
 119         NSK_COMPLAIN2("stackReferenceCallback: Local storage
was corrupted: %s ,\n\texpected value: %s\n",<br>
 120                          (char *)storage_ptr, storage_data);<br>
<br>
 162         NSK_COMPLAIN2("objectReferenceCallback: Local storage
was corrupted: %s ,\n\texpected value: %s\n",<br>
 163                          (char *)storage_ptr, storage_data);<br>
<br>
thanks,<br>
<br>
Chris<br>
<br>
On 10/19/18 1:49 PM, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF9BGBzitJLNKn0s08tRE1Brd2+-zC07nxpf3HtXwu2_D_+***@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<div dir="ltr">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>Sorry about the spam; forgot to add the subject :)</div>
<div><br>
</div>
<div>
<div dir="ltr">
<div>Here is the first of three webrevs to remove spaces
around (); I also removed any space after !.</div>
<div><br>
</div>
<div>When the change modified where future parameters
should be indented, I changed those too (such as <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html</a>)</div>
<div><br>
</div>
Webrev: <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/"
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/</a></div>
<div dir="ltr">Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8212535"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8212535</a><br
clear="all">
<br>
</div>
</div>
<div>Thanks!</div>
<div>Jc<br>
<br>
<div class="gmail_quote">
<div dir="ltr">On Fri, Oct 19, 2018 at 1:47 PM JC Beyler
&lt;<a href="mailto:***@google.com"
moz-do-not-send="true">***@google.com</a>&gt;
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div>Hi all,</div>
<div><br>
</div>
<div>Here is the first of three webrevs to
remove spaces around (); I also removed any
space after !.</div>
<div><br>
</div>
<div>When the change modified where future
parameters should be indented, I changed those
too (such as <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html</a>)</div>
<div><br>
</div>
Webrev: <a
href="https://bugs.openjdk.java.net/browse/JDK-8212535"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8212535</a></div>
<div dir="ltr">Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8212535"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8212535</a><br
clear="all">
<div><br>
</div>
<div dir="ltr"
class="gmail-m_-4710772905289707632gmail_signature">
<div dir="ltr">Let me know what you think,
<div>Jc</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</blockquote>
<p><br>
</p>
</body>
</html>
JC Beyler
2018-10-19 21:56:44 UTC
Permalink
Hi Chris,

Done!

Here is the newest version:
http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.01/

Thanks for the review!
Jc
Post by Chris Plummer
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.
%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
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
)
Webrev: http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8212535
Thanks!
Jc
Post by JC Beyler
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
)
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
Alex Menkov
2018-10-22 19:44:35 UTC
Permalink
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
Post by JC Beyler
Hi Chris,
Done!
http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.01/
<http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.01/>
Thanks for the review!
Jc
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
Post by JC Beyler
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
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
JC Beyler
2018-10-22 19:56:12 UTC
Permalink
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

The new webrev is here:
http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.02/

Thanks,
Jc
Post by Alex Menkov
Hi Jc,
Looks good to me.
Could you please update
if (!NSK_JVMTI_VERIFY(
- <jvmti_call>)) {
+ <jvmti_call>)) {
=>
if (!NSK_JVMTI_VERIFY(<jvmti_call>)) {
As all <jvmti_call>s there are quite short.
--alex
Post by JC Beyler
Hi Chris,
Done!
http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.01/
<http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.01/>
Thanks for the review!
Jc
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);
Post by JC Beyler
iterobjreachobj004.cpp: Can you fix the indentation of the second
line.
Post by JC Beyler
123 NSK_COMPLAIN2("Local storage was corrupted: %s
,\n\texpected value: %s\n",
124 (char *)storage_ptr,
storage_data);
Post by JC Beyler
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
Post by JC Beyler
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
Post by JC Beyler
Post by JC Beyler
<
http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html
Post by JC Beyler
)
Post by JC Beyler
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
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
Post by JC Beyler
Post by JC Beyler
<
http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html
Post by JC Beyler
)
Post by JC Beyler
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
Alex Menkov
2018-10-22 20:01:06 UTC
Permalink
This line was not changed in webrev.01 :)
Anyway looks good.

--alex
Post by JC Beyler
Hi Alex,
-            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))) {
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>
http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.02/
<http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.02/>
Thanks,
Jc
Hi Jc,
Looks good to me.
Could you please update
      if (!NSK_JVMTI_VERIFY(
-            <jvmti_call>)) {
+            <jvmti_call>)) {
=>
      if (!NSK_JVMTI_VERIFY(<jvmti_call>)) {
As all <jvmti_call>s there are quite short.
--alex
Post by JC Beyler
Hi Chris,
Done!
http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.01/
<http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.01/>
Post by JC Beyler
<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
     Hi JC,
     iterinstcls006.cpp: Can you fix the indentation of the second
line.
Post by JC Beyler
        98             NSK_COMPLAIN2("Local storage was corrupted: %s
     ,\n\texpected value: %s\n",
        99                              (char *)storage_ptr,
storage_data);
Post by JC Beyler
     iterobjreachobj004.cpp: Can you fix the indentation of the
second line.
Post by JC Beyler
       123             NSK_COMPLAIN2("Local storage was corrupted: %s
     ,\n\texpected value: %s\n",
       124                              (char *)storage_ptr,
storage_data);
Post by JC Beyler
     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.
Post by JC Beyler
        75         NSK_COMPLAIN2("heapRootCallback: Local storage was
     corrupted: %s ,\n\texpected value: %s\n",
        76                          (char *)storage_ptr,
storage_data);
Post by JC Beyler
       119         NSK_COMPLAIN2("stackReferenceCallback: Local
storage
Post by JC Beyler
     was corrupted: %s ,\n\texpected value: %s\n",
       120                          (char *)storage_ptr,
storage_data);
Post by JC Beyler
       162         NSK_COMPLAIN2("objectReferenceCallback: Local
storage
Post by JC Beyler
     was corrupted: %s ,\n\texpected value: %s\n",
       163                          (char *)storage_ptr,
storage_data);
Post by JC Beyler
     thanks,
     Chris
     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>)
http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/
<http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/>
Post by JC Beyler
     <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
         Hi all,
         Here is the first of three webrevs to remove spaces
around ();
Post by JC Beyler
         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>)
Post by JC Beyler
         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,
s***@oracle.com
2018-10-22 21:09:58 UTC
Permalink
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Alex,<br>
<br>
It looks good.<br>
<br>
Thanks!<br>
Serguei<br>
<br>
<br>
On 10/22/18 12:56, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF9BGBxOgCUzu=***@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">Hi Alex,
<div><br>
</div>
<div>Done, I left this one:</div>
<div><br>
</div>
<div>
<div>-            if (!NSK_JVMTI_VERIFY(</div>
<div>-                   
jvmti-&gt;IterateOverHeap(JVMTI_HEAP_OBJECT_EITHER,
heapObjectCallback, (void *)user_data))) {</div>
<div>+            if
(!NSK_JVMTI_VERIFY(jvmti-&gt;IterateOverHeap(</div>
<div>+                    JVMTI_HEAP_OBJECT_EITHER,
heapObjectCallback, (void *)user_data))) {</div>
</div>
<div><br>
</div>
<div>The whole length was at 126 characters, it seemed a
bit long:</div>
<div><a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverHeap/iterheap007/iterheap007.cpp.udiff.html"
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverHeap/iterheap007/iterheap007.cpp.udiff.html</a><br>
</div>
<div><br>
</div>
<div>The new webrev is here:</div>
<div><a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.02/"
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.02/</a><br>
</div>
<div><br>
</div>
<div>Thanks,</div>
<div>Jc</div>
</div>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Mon, Oct 22, 2018 at 12:44 PM Alex Menkov &lt;<a
href="mailto:***@oracle.com" target="_blank"
moz-do-not-send="true">***@oracle.com</a>&gt;
wrote:<br>
</div>
<blockquote class="gmail_quote">Hi Jc,<br>
<br>
Looks good to me.<br>
Could you please update<br>
nsk/jvmti/IterateOverHeap/iterheap007/iterheap007.cpp :<br>
<br>
      if (!NSK_JVMTI_VERIFY(<br>
-            &lt;jvmti_call&gt;)) {<br>
+            &lt;jvmti_call&gt;)) {<br>
<br>
=&gt;<br>
      if (!NSK_JVMTI_VERIFY(&lt;jvmti_call&gt;)) {<br>
<br>
As all &lt;jvmti_call&gt;s there are quite short.<br>
<br>
--alex<br>
<br>
<br>
On 10/19/2018 14:56, JC Beyler wrote:<br>
&gt; Hi Chris,<br>
&gt; <br>
&gt; Done!<br>
&gt; <br>
&gt; Here is the newest version:<br>
&gt; <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.01/"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.01/</a>
<br>
&gt; &lt;<a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.01/"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.01/</a>&gt;<br>
&gt; <br>
&gt; Thanks for the review!<br>
&gt; Jc<br>
&gt; <br>
&gt; On Fri, Oct 19, 2018 at 2:24 PM Chris Plummer &lt;<a
href="mailto:***@oracle.com" target="_blank"
moz-do-not-send="true">***@oracle.com</a> <br>
&gt; &lt;mailto:<a href="mailto:***@oracle.com"
target="_blank" moz-do-not-send="true">***@oracle.com</a>&gt;&gt;
wrote:<br>
&gt; <br>
&gt;     Hi JC,<br>
&gt; <br>
&gt;     iterinstcls006.cpp: Can you fix the indentation of
the second line.<br>
&gt; <br>
&gt;        98             NSK_COMPLAIN2("Local storage was
corrupted: %s<br>
&gt;     ,\n\texpected value: %s\n",<br>
&gt;        99                              (char
*)storage_ptr, storage_data);<br>
&gt; <br>
&gt;     iterobjreachobj004.cpp: Can you fix the indentation
of the second line.<br>
&gt; <br>
&gt;       123             NSK_COMPLAIN2("Local storage was
corrupted: %s<br>
&gt;     ,\n\texpected value: %s\n",<br>
&gt;       124                              (char
*)storage_ptr, storage_data);<br>
&gt; <br>
&gt;     iterreachobj002.cpp: You didn't align the arguments
like you have<br>
&gt;     elsewhere.<br>
&gt; <br>
&gt;       175<br>
&gt;   
 stackReferenceCallbackForSecondObjectsIteration(jvmtiHeapRootKind<br>
&gt;     root_kind,<br>
&gt;       176                          jlong     class_tag,<br>
&gt;       177                          jlong     size,<br>
&gt;       178                          jlong*    tag_ptr,<br>
&gt;       179                          jlong     thread_tag,<br>
&gt;       180                          jint      depth,<br>
&gt;       181                          jmethodID method,<br>
&gt;       182                          jint      slot,<br>
&gt;       183                          void*     user_data) {<br>
&gt; <br>
&gt;     iterreachobj004.cpp: Can you fix the indentation of
the second line.<br>
&gt; <br>
&gt;        75         NSK_COMPLAIN2("heapRootCallback: Local
storage was<br>
&gt;     corrupted: %s ,\n\texpected value: %s\n",<br>
&gt;        76                          (char *)storage_ptr,
storage_data);<br>
&gt; <br>
&gt;       119         NSK_COMPLAIN2("stackReferenceCallback:
Local storage<br>
&gt;     was corrupted: %s ,\n\texpected value: %s\n",<br>
&gt;       120                          (char *)storage_ptr,
storage_data);<br>
&gt; <br>
&gt;       162         NSK_COMPLAIN2("objectReferenceCallback:
Local storage<br>
&gt;     was corrupted: %s ,\n\texpected value: %s\n",<br>
&gt;       163                          (char *)storage_ptr,
storage_data);<br>
&gt; <br>
&gt;     thanks,<br>
&gt; <br>
&gt;     Chris<br>
&gt; <br>
&gt;     On 10/19/18 1:49 PM, JC Beyler wrote:<br>
&gt;&gt;     Hi all,<br>
&gt;&gt;<br>
&gt;&gt;     Sorry about the spam; forgot to add the subject
:)<br>
&gt;&gt;<br>
&gt;&gt;     Here is the first of three webrevs to remove
spaces around (); I<br>
&gt;&gt;     also removed any space after !.<br>
&gt;&gt;<br>
&gt;&gt;     When the change modified where future parameters
should be<br>
&gt;&gt;     indented, I changed those too (such as<br>
&gt;&gt;     <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html</a><br>
&gt;&gt;     &lt;<a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html</a>&gt;)<br>
&gt;&gt;<br>
&gt;&gt;     Webrev: <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/</a><br>
&gt;&gt;     &lt;<a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/</a>&gt;<br>
&gt;&gt;     Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8212535"
rel="noreferrer" target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8212535</a><br>
&gt;&gt;<br>
&gt;&gt;     Thanks!<br>
&gt;&gt;     Jc<br>
&gt;&gt;<br>
&gt;&gt;     On Fri, Oct 19, 2018 at 1:47 PM JC Beyler &lt;<a
href="mailto:***@google.com" target="_blank"
moz-do-not-send="true">***@google.com</a><br>
&gt;&gt;     &lt;mailto:<a href="mailto:***@google.com"
target="_blank" moz-do-not-send="true">***@google.com</a>&gt;&gt;
wrote:<br>
&gt;&gt;<br>
&gt;&gt;         Hi all,<br>
&gt;&gt;<br>
&gt;&gt;         Here is the first of three webrevs to remove
spaces around ();<br>
&gt;&gt;         I also removed any space after !.<br>
&gt;&gt;<br>
&gt;&gt;         When the change modified where future
parameters should be<br>
&gt;&gt;         indented, I changed those too (such as<br>
&gt;&gt;         <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html</a><br>
&gt;&gt;         &lt;<a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/8212535/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateOverObjectsReachableFromObject/iterobjreachobj002/iterobjreachobj002.cpp.udiff.html</a>&gt;)<br>
&gt;&gt;<br>
&gt;&gt;         Webrev: <a
href="https://bugs.openjdk.java.net/browse/JDK-8212535"
rel="noreferrer" target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8212535</a><br>
&gt;&gt;         Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8212535"
rel="noreferrer" target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8212535</a><br>
&gt;&gt;<br>
&gt;&gt;         Let me know what you think,<br>
&gt;&gt;         Jc<br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt; <br>
&gt; <br>
&gt; <br>
&gt; -- <br>
&gt; <br>
&gt; Thanks,<br>
&gt; Jc<br>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr" class="m_-5447876669232662063gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>
Hohensee, Paul
2018-10-23 00:23:30 UTC
Permalink
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-***@openjdk.java.net> on behalf of "***@oracle.com" <***@oracle.com>
Date: Monday, October 22, 2018 at 2:40 PM
To: JC Beyler <***@google.com>, "***@oracle.com" <***@oracle.com>
Cc: "serviceability-***@openjdk.java.net" <serviceability-***@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 <***@oracle.com<mailto:***@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
Post by JC Beyler
Hi Chris,
Done!
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
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
Post by JC Beyler
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
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
Martin Buchholz
2018-10-19 21:43:22 UTC
Permalink
Whenever I change only whitespace, I run a variant of

hg diff -wbB

to more easily see that it's truly only whitespace that has changed.
Post by JC Beyler
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)
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
Loading...