<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>
Thank you for the update!<br>
It looks good to me.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 10/8/18 14:53, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF9BGBz2_xKNSbD6wjDzKF+dWifdRgP5SWBjF_puEoce=j+***@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">Hi Serguei,<br>
<div><br>
</div>
<div>Normally, I try not to fix issues that already exist
before the change. For the space after the NULL for
example, that would get fixed by <a
href="https://bugs.openjdk.java.net/browse/JDK-8211335"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8211335</a>.
In this case, these are the only cases where this happens
with a NULL so I fixed them.</div> <div><br> </div> <div>For the space missing before jvmti->X, I fixed my
script to handle this correctly and add them
automatically.</div>
<div><br>
</div>
<div>Here is the new webrev:</div>
<div><br>
</div>
<div>Webrev: <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8211801/webrev.01"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8211801/webrev.01</a></div>
<div>Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8211801"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8211801</a></div>
<div><br>
</div>
<div>Thanks for the review!</div>
<div>Jc</div>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Mon, Oct 8, 2018 at 12:46 AM <a
href="mailto:***@oracle.com" target="_blank"
moz-do-not-send="true">***@oracle.com</a> <<a
href="mailto:***@oracle.com" target="_blank"
moz-do-not-send="true">***@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote">
<div>
<div
class="m_-3013947667350043943m_93347730047108057moz-cite-prefix">Hi
Jc,<br>
<br>
<br>
<a
class="m_-3013947667350043943m_93347730047108057moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejcbeyler/8211801/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP06/ap06t001/ap06t001.cpp.udiff.html"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/8211801/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP06/ap06t001/ap06t001.cpp.udiff.html</a><br> <br> <pre> if (!NSK_JNI_VERIFY(jni, (fid = <span class="m_-3013947667350043943m_93347730047108057removed">- NSK_CPP_STUB4(GetStaticFieldID, jni,</span> <span class="m_-3013947667350043943m_93347730047108057removed">- debugeeClass,</span> <span class="m_-3013947667350043943m_93347730047108057removed">- "thread",</span> <span class="m_-3013947667350043943m_93347730047108057removed">- THREAD_CLS_SIGNATURE)) != NULL )) {</span> <span class="m_-3013947667350043943m_93347730047108057new">+ jni->GetStaticFieldID(debugeeClass, "thread", THREAD_CLS_SIGNATURE)) != NULL )) {</span>
nsk_jvmti_setFailStatus();
break;
}
if (!NSK_JNI_VERIFY(jni, (localRefThread = <span class="m_-3013947667350043943m_93347730047108057removed">- NSK_CPP_STUB3(GetStaticObjectField, jni,</span> <span class="m_-3013947667350043943m_93347730047108057removed">- debugeeClass,</span> <span class="m_-3013947667350043943m_93347730047108057removed">- fid )) != NULL )) {</span> <span class="m_-3013947667350043943m_93347730047108057new">+ jni->GetStaticObjectField(debugeeClass, fid)) != NULL )) {</span>
NSK_COMPLAIN0("GetStaticObjectField returned NULL for 'thread' field value\n\n");
nsk_jvmti_setFailStatus();
break;
}
Could you, please, remove extra space after NULL.
</pre>
<br>
In many-many files (especially, in the foleder:
scenarios/capability):<br> <pre><span class="m_-3013947667350043943m_93347730047108057new">+ if (!NSK_JVMTI_VERIFY_CODE(JVMTI_ERROR_MUST_POSSESS_CAPABILITY,jvmti->SuspendThread(thread)))
... </span><span class="m_-3013947667350043943m_93347730047108057new">+ if (!NSK_JVMTI_VERIFY_CODE(JVMTI_ERROR_MUST_POSSESS_CAPABILITY,jvmti->ResumeThread(thread)))
... </span><span class="m_-3013947667350043943m_93347730047108057new">+ if (!NSK_JVMTI_VERIFY_CODE(JVMTI_ERROR_MUST_POSSESS_CAPABILITY,jvmti->InterruptThread(thread)))</span>
<span class="m_-3013947667350043943m_93347730047108057new"></span><span class="m_-3013947667350043943m_93347730047108057new"></span></pre>
<br>
There are a lot of cases like above where a space before
jvmti-> is missed.<br>
<br>
<br>
Otherwise, looks good.<br>
<br>
<br>
Thanks!<br>
Serguei<br>
<br>
<br>
On 10/5/18 16:50, JC Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>I continued the NSK_CPP_STUB removal with
this next webrev.<br>
</div>
<div><br>
</div>
<div>My apologies, this one is big for 50 files; I
can further cut it up if we prefer. It is
straight-forward though, since it is just doing
the same NSK_CPP_STUB removal.</div>
<div><br>
</div>
<div>Without further ado:</div>
<div>Webrev: <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8211801/webrev.00/"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8211801/webrev.00/</a></div>
<div>Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8211801"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8211801</a></div>
<div><br>
</div>
<div>
<div dir="ltr"
class="m_-3013947667350043943m_93347730047108057gmail-m_4350138340315380472gmail-m_-5052348339937521290gmail_signature">
<div dir="ltr">
<div>This does another 50 file batch. I did
the same for <a
href="https://bugs.openjdk.java.net/browse/JDK-8211782"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8211782</a>
so the scripts can be found there (and
added a comment in this bug to link to
that).</div>
<div><br>
</div>
<div>I've tested the modified changes on my
machine, all modified tests pass.<br>
</div>
<div><br>
</div>
Let me know what you think,
<div>Jc</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr" class="m_-3013947667350043943gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>