<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">One more suggestion for simplification:<br>
<pre> 56 char *generic = NULL;
57
58 (*jvmti)->GetClassSignature(jvmti, klass, &signature, &generic);
59 </pre>
 The value returned in generic is not used.<br>
 You can pass NULL in place of @generic and get rid of line #56.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 8/24/18 09:41, <a class="moz-txt-link-abbreviated" href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:6e07e466-d89f-ad58-8f6e-e538280cca4a@oracle.com">
<div class="moz-cite-prefix">Hi Jc,<br>
<br>
It looks good in general, thank you for taking care about this!<br>
A couple of comments though.<br>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejcbeyler/8203356/webrev.00/test/hotspot/jtreg/serviceability/jvmti/VMEvent/libVMEventTest.c.html"
moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/8203356/webrev.00/test/hotspot/jtreg/serviceability/jvmti/VMEvent/libVMEventTest.c.html</a><br>
<br>
<pre> 58 (*jvmti)->GetClassSignature(jvmti, klass, &signature, &generic);
59
60 if (JNI_ENV_PTR(jni)->ExceptionOccurred(JNI_ENV_ARG(jni))) {
61 JNI_ENV_PTR(jni)->FatalError(
62 JNI_ENV_ARGS2(jni, "Failed during the GetClassSignature call"));
63 }
The JVMTI error code, returned by GetClassSignature has to be checked, not JNI ExceptionOccurred.
Also, I'd suggest to check for signature to be non-NULL.
Also, the indent in the VMEventRecursionTest.java has to be 4 (as we normally use for java code), not 2.
Thanks,
Serguei
</pre>
<br>
On 8/22/18 16:20, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF9BGBzDR8eRvRGF5cU=ucm1nHhQo27iL5JdyD8J=ga3-Gn5CQ@mail.gmail.com">
<div dir="ltr">Hi all,<br>
<div><br>
</div>
<div>Would anyone want to look at this change? It helps fix a
minor bug if someone provokes a VM allocation during a VM
Allocation Event.</div>
<div><br>
</div>
<div>Webrev:Â <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8203356/webrev.00/"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8203356/webrev.00/</a><br>
<div>Bug:Â <a
href="https://bugs.openjdk.java.net/browse/JDK-8203356"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8203356</a></div>
</div>
<div><br>
</div>
<div>Thanks!</div>
<div>Jc</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Thu, Aug 2, 2018 at 12:46 PM JC Beyler <<a
href="mailto:jcbeyler@google.com" moz-do-not-send="true">jcbeyler@google.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>(Renaming the thread that did not have the RFR in
front of the subject, I apologize)</div>
<div><br>
</div>
<div>Could someone review this change:</div>
<div><br>
</div>
<div>Webrev:Â <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8203356/webrev.00/"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8203356/webrev.00/</a><br>
<div>Bug:Â <a
href="https://bugs.openjdk.java.net/browse/JDK-8203356"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8203356</a></div>
<div><br>
</div>
<div>Basically, if during a callback from
a VMObjectAlloc event, the user provokes a clone, the
code would send a new callback and you can recurse
infinitely.</div>
<div><br>
</div>
<div>I added a test that fails without the fix and
passes now.</div>
<div><br>
</div>
<div dir="ltr"
class="m_5265400520167476713gmail-m_1857162544524225239gmail_signature">
<div dir="ltr">Thanks,
<div>Jc</div>
</div>
</div>
</div>
</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>
</blockquote>
<br>
</body>
</html>