<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hi Valerie, looks pretty good. I only had a few minor comments.<br>
</p>
<ul>
<li>pkcs11wrapper.h</li>
<ul>
<li>210-212: Do you intend to leave these trace macros in there
but commented? It seems like they could be safely left
uncommented in case you needed to use them for debugging.</li>
<li>388: This looks like a duplicate of line 387.</li>
</ul>
<li>libj2pkcs11.c</li>
<ul>
<li>271/277 (and others) - If you know you're going to zero the
memory right off the bat you could use calloc() rather than
malloc and get the benefit of the memory being zeroed
automatically. Just a suggestion, there's nothing wrong with
the code as it is.</li>
<li>395: nit - extra space between ckpdate and ";"</li>
<li>843: unnecessary double-parenthesis</li>
</ul>
</ul>
<p>--Jamil<br>
</p>
<div class="moz-cite-prefix">On 8/6/19 7:28 PM, Valerie Peng wrote:<br>
</div>
<blockquote type="cite"
cite="mid:86ddf8f4-7fd4-4e6d-2364-043d351cb4bc@oracle.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<p>Anyone have spare cycles for reviewing this? <br>
</p>
<p>The current PKCS11 JNI code for handling native mechanism and
its parameters is a bit too all over the place. To make the
tracing and verification easier, I consolidated the memory
deallocation to the freeCKMechanismPtr(...) method in p11_util.c
(some was in p11_keymgmt.c). <br>
</p>
<p>Also, fixed the j<u>XXX</u>ToCK<u>XXXX</u> methods in
p11_convert.c to allocate the memory inside each method and
return NULL upon error and make sure allocated memories are
free'd upon any failure.<br>
</p>
<p>Bug: <a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8228835"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8228835</a></p>
<p>Webrev: <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~valeriep/8228835/webrev.00/"
moz-do-not-send="true">http://cr.openjdk.java.net/~valeriep/8228835/webrev.00/</a></p>
<p>Mach5 run is clean.</p>
Thanks,<br>
Valerie<br>
</blockquote>
</body>
</html>