<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Hi Semyon,<br>
    <br>
    <div class="moz-cite-prefix">On 26/09/2017 21:58, Semyon Sadetsky
      wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:59CABF7D.10201@oracle.com">
      <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
      Hi Alexey,<br>
      <br>
      On 9/26/2017 12:29 PM, Alexey Ivanov wrote:<br>
      <blockquote
        cite="mid:9331b1e4-dea0-5f2e-3afd-a9e4a8010fc3@oracle.com"
        type="cite">
        <meta http-equiv="Content-Type" content="text/html;
          charset=utf-8">
        Hi Semyon,<br>
        <br>
        ShellFolder2.cpp<br>
        <pre><span class="changed">944             pIcon->Extract(szBuf, index, &hIcon, <b>0</b>, sz);</span></pre>
        I think 0 should really be <code>NULL</code>.<br>
      </blockquote>
      Ok, but both represent the null pointer in Win32.<br>
    </blockquote>
    <br>
    Yes, I know. Yet NULL makes it clear that it's a null-pointer rather
    than an index, size…<br>
    It's still zero in the latest review.<br>
    <br>
    <blockquote type="cite" cite="mid:59CABF7D.10201@oracle.com"> <br>
      <blockquote
        cite="mid:9331b1e4-dea0-5f2e-3afd-a9e4a8010fc3@oracle.com"
        type="cite"> The result of the call is ignored now. Is it
        intentional?<br>
      </blockquote>
      Yes, it has been working the same way before the fix. The function
      returns the Icon reference which is 0 in case of OS API call
      error.<br>
      <br>
      <blockquote
        cite="mid:9331b1e4-dea0-5f2e-3afd-a9e4a8010fc3@oracle.com"
        type="cite"> <br>
        Win32ShellFolder2.java<br>
        <pre><span class="changed"><span class="changed">1010     private static Image makeIcon(long hIcon, int bsize) {
</span></span></pre>
        <code>bsize</code> was called <code>baseSize</code> previously,
        and it conveyed the meaning better, didn't it?<br>
        <br>
        <pre><span class="new">1043                 if(hIcon <= 0) {</span>
<span class="new">1044                     if (isDirectory()) {</span>
<span class="new">1045                         return getShell32Icon(4, size);</span>
<span class="new">1046                     } else {</span>
<span class="new">1047                         return getShell32Icon(1, size);</span>
<span class="new">1048                     }</span></pre>
        I guess I understand what hides behind 4 and 1: generic folder
        and generic file icon ids. Would declaring these numbers as
        constants improve readability?<br>
      </blockquote>
      Ok.<br>
      <blockquote
        cite="mid:9331b1e4-dea0-5f2e-3afd-a9e4a8010fc3@oracle.com"
        type="cite"> <br>
        <code>getIcon(int size)</code> and <code>getIcon(boolean
          getLargeIcon)</code> are somewhat similar. The latter provides
        additional caching. Can it be simplified to re-use portions of
        new <code>getIcon(int size)</code>?<br>
      </blockquote>
      It has additional difference because of query for a fixed icon
      size from another API call.  It's better to leave it as it is.<br>
    </blockquote>
    <br>
    Okay.<br>
    <br>
    <br>
    Regards,<br>
    Alexey<br>
    <br>
    <blockquote type="cite" cite="mid:59CABF7D.10201@oracle.com"> <br>
      <a class="moz-txt-link-freetext"
        href="http://cr.openjdk.java.net/%7Essadetsky/8182043/webrev.02/"
        moz-do-not-send="true">http://cr.openjdk.java.net/~ssadetsky/8182043/webrev.02/</a><br>
      <br>
      --Semyon<br>
      <blockquote
        cite="mid:9331b1e4-dea0-5f2e-3afd-a9e4a8010fc3@oracle.com"
        type="cite"> <br>
        <br>
        Regards,<br>
        Alexey<br>
        <br>
        <div class="moz-cite-prefix">On 22/09/2017 22:05, Semyon
          Sadetsky wrote:<br>
        </div>
        <blockquote type="cite"
          cite="mid:125d7526-1b85-d950-41c6-fbadebbde114@oracle.com">
          <meta http-equiv="Content-Type" content="text/html;
            charset=utf-8">
          <p><SNIP><br>
          </p>
        </blockquote>
      </blockquote>
    </blockquote>
  </body>
</html>