<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>Hi Jamil,</p>
    <p>Thanks for the review, webrev updated at
      <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~valeriep/8228835/webrev.01/">http://cr.openjdk.java.net/~valeriep/8228835/webrev.01/</a></p>
    <p>More replies in line below.<br>
    </p>
    <div class="moz-cite-prefix">On 8/8/2019 5:30 PM, Jamil Nimeh wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:6e3edb28-18a1-362f-273e-5a87d2c5339f@oracle.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <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>
      </ul>
    </blockquote>
    <p>Yes, I have used those trace macros quite extensively, so I left
      them in and commented out. More convenient this way.</p>
    <p>Removed line 388.<br>
    </p>
    <blockquote type="cite"
      cite="mid:6e3edb28-18a1-362f-273e-5a87d2c5339f@oracle.com">
      <ul>
        <ul>
        </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>
    </blockquote>
    <p>Fixed.</p>
    <p>Mach5 run is clean. Please let me know if you have more comments.<br>
    </p>
    <p>Thanks,</p>
    <p>Valerie<br>
    </p>
    <blockquote type="cite"
      cite="mid:6e3edb28-18a1-362f-273e-5a87d2c5339f@oracle.com">
      <ul>
        <ul>
        </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>
    </blockquote>
  </body>
</html>