<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Nope, no other comments, looks good to me.<br>
    <br>
    --Jamil<br>
    <br>
    <div class="moz-cite-prefix">On 8/13/2019 2:21 PM, Valerie Peng
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:de89faa4-01af-105e-d823-b97d0875b9cc@oracle.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <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/"
          moz-do-not-send="true">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>
    </blockquote>
    <br>
  </body>
</html>