<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>
It is Okay if it is covered by different bugs.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 11/7/18 13:59, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:***@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">Hi Serguei,<br>
<div><br>
</div>
<div>I have <a
href="https://bugs.openjdk.java.net/browse/JDK-8212960"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8212960</a>
open for that. I like doing things in small contained
steps for reviewing and sanity checking. But if you want,
I can run a script to fix all these cases now and resent a
RFR.</div>
<div>I created <a
href="https://bugs.openjdk.java.net/browse/JDK-8213499"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8213499</a>
for the arguments.</div>
<div><br>
</div>
<div>Let me know what you prefer, I can see the pros/cons of
both ways :-),</div>
<div>Jc</div>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Wed, Nov 7, 2018 at 1:01 PM <a
href="mailto:***@oracle.com"
moz-do-not-send="true">***@oracle.com</a> <<a
href="mailto:***@oracle.com"
moz-do-not-send="true">***@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote">
<div>
<div class="m_8217570621416410833moz-cite-prefix">Hi Jc,<br>
<br>
+1 LGTM.<br>
<br>
One question:<br>
Would it be convenient at the same time to fix missing
spaces around commas and comparisons in loops and
conditions.<br>
One more case is function call arguments but I did not
see any instances of it.<br>
<br>
Some examples:<br>
<br>
<a class="m_8217570621416410833moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/Breakpoint/breakpoint001/breakpoint001.cpp.udiff.html"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/Breakpoint/breakpoint001/breakpoint001.cpp.udiff.html</a><br>
<pre><span class="m_8217570621416410833removed">- for(i=0; i<METH_NUM; i++)</span>
<span class="m_8217570621416410833new">+ for (i=0; i<METH_NUM; i++)</span></pre>
<br>
<a class="m_8217570621416410833moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/ClassLoad/classload001/classload001.cpp.udiff.html"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/ClassLoad/classload001/classload001.cpp.udiff.html</a><br>
<pre><span class="m_8217570621416410833removed">- for(i=0; i<EXP_SIG_NUM; i++)</span>
<span class="m_8217570621416410833new">+ for (i=0; i<EXP_SIG_NUM; i++)</span>
clsEvents[i] = 0;
<span class="m_8217570621416410833removed">- for(i=0; i<UNEXP_SIG_NUM; i++)</span>
<span class="m_8217570621416410833new">+ for (i=0; i<UNEXP_SIG_NUM; i++)</span></pre>
<br>
<br>
<a class="m_8217570621416410833moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/stress/jni/libjnistress004.cpp.udiff.html"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/stress/jni/libjnistress004.cpp.udiff.html</a><br>
<br>
There are many cases like below:<br>
<pre><span class="m_8217570621416410833removed">- for(i=0;i<DIGESTLENGTH;i++) {</span>
<span class="m_8217570621416410833new">+ for (i=0;i<DIGESTLENGTH;i++) {</span></pre>
. . .<br>
<pre><span class="m_8217570621416410833removed">- for(i=0;i<len;i++)</span>
<span class="m_8217570621416410833removed">- if(ch[i]!=tmp[i]) {</span>
<span class="m_8217570621416410833new">+ for (i=0;i<len;i++)</span>
<span class="m_8217570621416410833new">+ if (ch[i]!=tmp[i]) {</span></pre>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 11/7/18 12:23, Chris Plummer wrote:<br>
</div>
<blockquote type="cite">Hi JC. <br>
<br>
In Callbacks.cpp you missed the needed extra indent on the
lines following the "if": <br>
<br>
406 if
(!NSK_JVMTI_VERIFY(jvmti->GetFieldName(targetClass, <br>
407 targetFields[field], <br>
408 &objects_info[object].fields[field].name, <br>
409 &objects_info[object].fields[field].signature, <br>
410 NULL))) <br>
<br>
481 if
((objects_info[object].fields[field].primitive &&
!objects_info[object].collected) <br>
482 ||
!objects_info[object].fields[field].collected) { <br>
<br>
Otherwise looks good. I don't need to see another webrev.
<br>
<br>
thanks, <br>
<br>
Chris <br>
<br>
On 11/7/18 11:20 AM, JC Beyler wrote: <br>
<blockquote type="cite">Hi all, <br>
<br>
Could I get a review for a relatively straight-forward
space transformation webrev? This adds spaces between
keywords and '(' and fixes the case "){" across C++
files in vmTestbase. <br>
<br>
Webrev: <a
class="m_8217570621416410833moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212939/webrev.00/</a>
<a class="m_8217570621416410833moz-txt-link-rfc2396E"
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/"
target="_blank" moz-do-not-send="true"><http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/></a>
<br>
Bug: <a
class="m_8217570621416410833moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8212939"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8212939</a>
<br>
<br>
Thanks! <br>
Jc <br>
</blockquote>
<br>
<br>
<br>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr" class="gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>