<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=us-ascii"><meta name=Generator content="Microsoft Word 15 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
        {font-family:PMingLiU;
        panose-1:2 1 6 1 0 1 1 1 1 1;}
@font-face
        {font-family:Tunga;
        panose-1:0 0 4 0 0 0 0 0 0 0;}
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:Consolas;
        panose-1:2 11 6 9 2 2 4 3 2 4;}
@font-face
        {font-family:"\@PMingLiU";}
@font-face
        {font-family:"Times New Roman \,serif";
        panose-1:0 0 0 0 0 0 0 0 0 0;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;
        color:black;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:#0563C1;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:#954F72;
        text-decoration:underline;}
p
        {mso-style-priority:99;
        mso-margin-top-alt:auto;
        margin-right:0in;
        mso-margin-bottom-alt:auto;
        margin-left:0in;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;
        color:black;}
pre
        {mso-style-priority:99;
        mso-style-link:"HTML Preformatted Char";
        margin:0in;
        margin-bottom:.0001pt;
        font-size:10.0pt;
        font-family:"Courier New";
        color:black;}
span.HTMLPreformattedChar
        {mso-style-name:"HTML Preformatted Char";
        mso-style-priority:99;
        mso-style-link:"HTML Preformatted";
        font-family:Consolas;
        color:black;}
span.new
        {mso-style-name:new;}
span.changed
        {mso-style-name:changed;}
span.EmailStyle22
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:windowtext;}
span.EmailStyle23
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
span.EmailStyle24
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
span.EmailStyle25
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
span.EmailStyle26
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
span.EmailStyle27
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
span.EmailStyle28
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
span.EmailStyle29
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
span.EmailStyle30
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
span.EmailStyle31
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
span.EmailStyle32
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
span.EmailStyle33
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@page WordSection1
        {size:8.5in 11.0in;
        margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body bgcolor=white lang=EN-US link="#0563C1" vlink="#954F72"><div class=WordSection1><p class=MsoNormal><span style='color:#1F497D'>Hi Prasanta, I hope I have answered your questions satisfactorily. Please let me know if you have any more questions.<o:p></o:p></span></p><p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='color:#1F497D'>Thanks and regards,<o:p></o:p></span></p><p class=MsoNormal><span style='color:#1F497D'>Shashi<o:p></o:p></span></p><p class=MsoNormal><a name="_MailEndCompose"><span style='color:#1F497D'><o:p> </o:p></span></a></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='color:windowtext'>From:</span></b><span style='color:windowtext'> Shashidhara Veerabhadraiah <br><b>Sent:</b> Friday, February 15, 2019 12:37 PM<br><b>To:</b> Prasanta Sadhukhan <prasanta.sadhukhan@oracle.com>; Philip Race <philip.race@oracle.com><br><b>Cc:</b> 2d-dev@openjdk.java.net<br><b>Subject:</b> RE: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal><span style='color:#1F497D'>Hi Prasanta, The parameters for the function does not tells the order and is not implied by the way function definition is and more over this function is called in an interval of minimum refresh time(4 mins). Rather than adding overhead of confusion since it is not implied by the function I think it is safe to keep it that way. Depending on reviewer’s comments is also not the right way to depend on. The current definition avoids that but with a small overhead and I think is not too much of a burden to bear.<o:p></o:p></span></p><p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='color:#1F497D'>Thanks and regards,<o:p></o:p></span></p><p class=MsoNormal><span style='color:#1F497D'>Shashi<o:p></o:p></span></p><p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='color:windowtext'>From:</span></b><span style='color:windowtext'> Prasanta Sadhukhan <br><b>Sent:</b> Friday, February 15, 2019 12:28 PM<br><b>To:</b> Shashidhara Veerabhadraiah <<a href="mailto:shashidhara.veerabhadraiah@oracle.com">shashidhara.veerabhadraiah@oracle.com</a>>; Philip Race <<a href="mailto:philip.race@oracle.com">philip.race@oracle.com</a>><br><b>Cc:</b> <a href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br><b>Subject:</b> Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><p>Hi Shashi,<span style='font-size:12.0pt'><o:p></o:p></span></p><p>In this case, we know doCompare() is called from one location only [and also is not a public method to be called by user] and the check I mentioned is redundant. It's in while(true) loop so any optimzation we can do to avoid unnecessary checks will be good in my opinion.<o:p></o:p></p><p>If someone changed the doCompare() call without seeing the implication or giving thought, then he has to face the repurcussions, but I guess reviewers would be able to catch that beforehand.<o:p></o:p></p><p>Regards<o:p></o:p></p><p>Prasanta<o:p></o:p></p><div><p class=MsoNormal>On 15-Feb-19 12:17 PM, Shashidhara Veerabhadraiah wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span style='color:#1F497D'>Hi Prasanta, doCompare(str1, str2) is a different function and one should not define a function based on how it is going to be called!! What if someone changed the caller to this: </span><span style='font-size:10.0pt;font-family:Consolas'>doCompare(currentRemotePrinters, prevRemotePrinters)</span><span style='color:#1F497D'>. There is no restriction if one calls like this. Here the function is taking 2 strings and it does not say which one should be passed at what position. Probably if the function takes different parameters then it sets an automatic rule on which parameter needs to be passed at which position but otherwise function definition should take care this.</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Hope you agree with me.</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Thanks and regards,</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Shashi</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='color:windowtext'>From:</span></b><span style='color:windowtext'> Prasanta Sadhukhan <br><b>Sent:</b> Friday, February 15, 2019 12:11 PM<br><b>To:</b> Phil Race <a href="mailto:philip.race@oracle.com"><philip.race@oracle.com></a>; Shashidhara Veerabhadraiah <a href="mailto:shashidhara.veerabhadraiah@oracle.com"><shashidhara.veerabhadraiah@oracle.com></a><br><b>Cc:</b> <a href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br><b>Subject:</b> Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732</span><o:p></o:p></p></div></div><p class=MsoNormal> <o:p></o:p></p><p>Hi Shashi,<o:p></o:p></p><p class=MsoNormal>I have one comment:<br><span style='font-size:10.0pt;font-family:Consolas'>doCompare(prevRemotePrinters, currentRemotePrinters)</span> is only called from run() method when "prevRemotePrinters" is already checked to be not null [<b><span style='font-size:10.0pt;font-family:Consolas;color:blue'>if (prevRemotePrinters != null && prevRemotePrinters.length > 0) ]</span></b><br>so I see no point in checking "str1" [which is prevRemotePrinters] to be null in doCompare(). I guess instead of<br><br><span class=new><b><span style='font-size:10.0pt;font-family:Consolas;color:blue'>413             if (str1 == null && str2 == null) {</span></b></span><o:p></o:p></p><p class=MsoNormal><span class=new><b><span style='font-size:10.0pt;font-family:Consolas;color:blue'>414                 return false;</span></b></span><o:p></o:p></p><p class=MsoNormal><span class=new><b><span style='font-size:10.0pt;font-family:Consolas;color:blue'>415             } else if (str1 == null || str2 == null) {</span></b></span><o:p></o:p></p><p class=MsoNormal><span class=new><b><span style='font-size:10.0pt;font-family:Consolas;color:blue'>416                 return true;</span></b></span><o:p></o:p></p><p class=MsoNormal><span class=new><b><span style='font-size:10.0pt;font-family:Consolas;color:blue'>417             }</span></b></span><o:p></o:p></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:Consolas'>you can have</span><o:p></o:p></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:Consolas'>if (str2 == null) </span><o:p></o:p></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:Consolas'>    return true; </span><o:p></o:p></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:Consolas'> </span><o:p></o:p></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:Consolas'>Regards</span><o:p></o:p></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:Consolas'>Prasanta</span><o:p></o:p></p><div><p class=MsoNormal>On 15-Feb-19 2:48 AM, Phil Race wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal style='margin-bottom:12.0pt'>+1 .. remember to use the current bug synopsis in the push comment<br><br>ie : "[Windows] Exception if no printers are installed."<br><br>-phil.<o:p></o:p></p><div><p class=MsoNormal>On 2/11/19 12:39 AM, Shashidhara Veerabhadraiah wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span style='color:#1F497D'>Hi Phil, Here is the new Webrev fixing those comments:</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'><a href="http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.06/">http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.06/</a></span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Thanks and regards,</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Shashi</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='color:windowtext'>From:</span></b><span style='color:windowtext'> Philip Race <br><b>Sent:</b> Saturday, February 9, 2019 2:25 AM<br><b>To:</b> Shashidhara Veerabhadraiah <a href="mailto:shashidhara.veerabhadraiah@oracle.com"><shashidhara.veerabhadraiah@oracle.com></a><br><b>Cc:</b> Prasanta Sadhukhan <a href="mailto:prasanta.sadhukhan@oracle.com"><prasanta.sadhukhan@oracle.com></a>; <a href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br><b>Subject:</b> Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732</span><o:p></o:p></p></div></div><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal><br>413             if (str1 == null && str2 == null) {<br> 414                 return false;<br> 415             } else if ((str1 == null && str2 != null) || (str2 == null && str1 != null)) {<br> 416                 return true;<br> 417             }<br><br>When we get to 415 we already know at least one of str1 or str2 is non-null so 415 can just be<br><br>} else if (str1 == null || str2 == null) {<br><br>-phil.<br> <br>On 2/6/19, 12:31 AM, Shashidhara Veerabhadraiah wrote: <o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span style='color:#1F497D'>Hi Phil, Here is the updated Webrev fixing those comments:</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'><a href="http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.05/">http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.05/</a></span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Thanks and regards,</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Shashi</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='color:windowtext'>From:</span></b><span style='color:windowtext'> Phil Race <br><b>Sent:</b> Thursday, January 31, 2019 4:36 AM<br><b>To:</b> Shashidhara Veerabhadraiah <a href="mailto:shashidhara.veerabhadraiah@oracle.com"><shashidhara.veerabhadraiah@oracle.com></a><br><b>Cc:</b> Prasanta Sadhukhan <a href="mailto:prasanta.sadhukhan@oracle.com"><prasanta.sadhukhan@oracle.com></a>; <a href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br><b>Subject:</b> Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732</span><o:p></o:p></p></div></div><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>Sorry .. this got lost .. <br><br>I don't understand this line :<o:p></o:p></p><pre><span class=changed> 253     jobjectArray emptyArray = env->NewObjectArray(1, clazz, NULL);</span><o:p></o:p></pre><p class=MsoNormal><br>This is returning an array of length 1 and element 0 is NULL.<br>I think you want <o:p></o:p></p><pre><span class=changed>env->NewObjectArray(0, clazz, NULL);</span><o:p></o:p></pre><pre><span class=changed> </span><o:p></o:p></pre><pre><span class=changed>and I don't see why you need to create it there</span><o:p></o:p></pre><pre><span class=changed>instead you can just have</span><o:p></o:p></pre><pre><span class=changed> </span><o:p></o:p></pre><pre><span class=new>304     if (nameArray != NULL) {</span><o:p></o:p></pre><pre> 305       return nameArray;<o:p></o:p></pre><pre><span class=new> 306     } else {</span><o:p></o:p></pre><pre><span class=new> 307       return </span><span class=changed>env->NewObjectArray(0, clazz, NULL);</span><o:p></o:p></pre><pre><span class=new> 308     }</span><o:p></o:p></pre><pre><span class=new> </span><o:p></o:p></pre><pre><span class=new> 449                 if (prevRemotePrinters != null) {</span><o:p></o:p></pre><pre><span class=new> </span><o:p></o:p></pre><pre><span class=new>shouldn't this be</span><o:p></o:p></pre><pre><span class=new> 449                 if (prevRemotePrinters != null && prevRemotePrinters.length > 0) {</span><o:p></o:p></pre><pre><span class=new> </span><o:p></o:p></pre><pre><span class=new>?</span><o:p></o:p></pre><pre><span class=new> </span><o:p></o:p></pre><pre><span class=new>-phil.</span><o:p></o:p></pre><div><p class=MsoNormal>On 12/10/18 10:19 PM, Shashidhara Veerabhadraiah wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span style='color:#1F497D'>Hi Phil, I have updated with small code updates:</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'><a href="http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.04/">http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.04/</a></span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Thanks and regards,</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Shashi</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='color:windowtext'>From:</span></b><span style='color:windowtext'> Shashidhara Veerabhadraiah <br><b>Sent:</b> Monday, December 10, 2018 3:19 PM<br><b>To:</b> Philip Race <a href="mailto:philip.race@oracle.com"><philip.race@oracle.com></a><br><b>Cc:</b> Prasanta Sadhukhan <a href="mailto:prasanta.sadhukhan@oracle.com"><prasanta.sadhukhan@oracle.com></a>; <a href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br><b>Subject:</b> RE: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732</span><o:p></o:p></p></div></div><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Hi Phil, Please find the updated Webrev.</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'><a href="http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.03/">http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.03/</a></span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Thanks and regards,</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Shashi</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='color:windowtext'>From:</span></b><span style='color:windowtext'> Philip Race <br><b>Sent:</b> Friday, December 7, 2018 8:54 PM<br><b>To:</b> Shashidhara Veerabhadraiah <<a href="mailto:shashidhara.veerabhadraiah@oracle.com">shashidhara.veerabhadraiah@oracle.com</a>><br><b>Cc:</b> Prasanta Sadhukhan <<a href="mailto:prasanta.sadhukhan@oracle.com">prasanta.sadhukhan@oracle.com</a>>; <a href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br><b>Subject:</b> Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732</span><o:p></o:p></p></div></div><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>First off, I think the high order bit is that if a non-null array reference is returned there are NO<br>null String entries in it. Whether a zero length or null array is returned when there are no printers<br>is the less important issue.<br><br>However an empty array also tells you there are no printers, and you are less likely to get an NPE from that.<br>It is arguably easier for the caller, if they don't need the extra null check first.<br>FWIW the javax.print public APIs return zero length arrays instead of null and applications seem to survive :-)<br><br>I don't know what you mean by :<br>> <span style='color:#1F497D'>(And anyway we need to handle the first null string reference)?<br><br>If you are referring to a small matter of coding in the native layer, then that is not an insurmountable problem.<br><br>Basically if there are problems getting names or whatever in the native layer, handle it THERE, don't make<br>everyone else have to deal with it.<br><br>One caveat: JNI calls can theoretically fail due to OOME .. in such a case we are doomed and<br>the main goal is to not crash in native and to obey all JNI rules. What is returned in that case<br>can be a null array reference and an NPE in a Java-level user of that reference in such a case is not a big deal.<br></span><br>-phil<br><br>On 12/6/18, 8:10 PM, Shashidhara Veerabhadraiah wrote: <o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span style='color:#1F497D'>Hi Phil, Is it advisable to return zero length array from native? The null return from native is telling the caller that there are no printers connected to the system. To tell this should we allocate a zero length array containing null back to the caller(And anyway we need to handle the first null string reference)? Handling the null on the caller isn’t an easier option?</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Thanks and regards,</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Shashi</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='color:windowtext'>From:</span></b><span style='color:windowtext'> Phil Race <br><b>Sent:</b> Thursday, December 6, 2018 2:35 AM<br><b>To:</b> Shashidhara Veerabhadraiah <a href="mailto:shashidhara.veerabhadraiah@oracle.com"><shashidhara.veerabhadraiah@oracle.com></a>; Prasanta Sadhukhan <a href="mailto:prasanta.sadhukhan@oracle.com"><prasanta.sadhukhan@oracle.com></a>; <a href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br><b>Subject:</b> Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732</span><o:p></o:p></p></div></div><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal style='margin-bottom:12.0pt'>But what I am saying is we should not return a NULL string reference<br>from native. You are still allowing that and then having to handle<br>it in Java code.<br><br>And FWIW you can return a zero length array as well so there<br>isn't a NULL array reference to deal with either.<br><br>-phil.<o:p></o:p></p><div><p class=MsoNormal>On 12/3/18 8:29 AM, Shashidhara Veerabhadraiah wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span style='color:#1F497D'>Hi Phil, There were 2 problems earlier. One is that, from the native it is possible to have no printers being associated with the system(causing to return null reference) and other problem in the implementation was that there may be no network printers and still return a valid array reference containing a null string. The first problem is fixed by handling null references returned from this function and other problem was that earlier there were different base conditions, for updating the reference and use it to create the printer name array which could produce a valid reference pointing to null string. Here is the updated Webrev which fixes these problems:</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'><a href="http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.02/">http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.02/</a></span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>The big problem was that I was not able to reproduce this problem neither at my end nor at the systems used for PIT testing. Only Sergey had produced this NPE till now.</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Thanks and regards,</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Shashi</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='color:windowtext'>From:</span></b><span style='color:windowtext'> Phil Race <br><b>Sent:</b> Wednesday, November 28, 2018 11:19 PM<br><b>To:</b> Shashidhara Veerabhadraiah <a href="mailto:shashidhara.veerabhadraiah@oracle.com"><shashidhara.veerabhadraiah@oracle.com></a>; Prasanta Sadhukhan <a href="mailto:prasanta.sadhukhan@oracle.com"><prasanta.sadhukhan@oracle.com></a>; <a href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br><b>Subject:</b> Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732</span><o:p></o:p></p></div></div><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal style='margin-bottom:12.0pt'>I am not understanding you. I thought the problem to be we got an array of (say) 3 values<br>(ie printer names) returned from native where some or all of the *values* were NULL.<br>And I am saying we should in such a case in the native code, before returning,<br>remove from the returned array all values that are NULL.<br>That could result in an empty (zero length) array being returned from native but<br>no null names .. <br><br>-phil.<o:p></o:p></p><div><p class=MsoNormal>On 11/26/18 10:23 PM, Shashidhara Veerabhadraiah wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span style='color:#1F497D'>The windows function EnumPrinters() returns a value that could be zero or greater. If it is zero we have no other option but to return null from the function. I do not think there is a way where we can always make sure we get a non-zero value considering the various system scenarios. So we should handle the null return values as well in the caller of this function I think.</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Here is the updated Webrev for test fix:</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'><a href="http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.01/">http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.01/</a></span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Thanks and regards,<br>Shashi</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='color:windowtext'>From:</span></b><span style='color:windowtext'> Phil Race <br><b>Sent:</b> Tuesday, November 27, 2018 1:52 AM<br><b>To:</b> Prasanta Sadhukhan <a href="mailto:prasanta.sadhukhan@oracle.com"><prasanta.sadhukhan@oracle.com></a>; Shashidhara Veerabhadraiah <a href="mailto:shashidhara.veerabhadraiah@oracle.com"><shashidhara.veerabhadraiah@oracle.com></a>; <a href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br><b>Subject:</b> Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732</span><o:p></o:p></p></div></div><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal style='margin-bottom:12.0pt'>[Removed swing-dev as this as nothing to do with swing].<br><br>You mention in the bug eval that you don't need a new test because this<br>is already covered by the test for 8153732. If that is the case then this<br>bugid should be added to that test.<br>Although it also looks like there are plenty of tests that provoke this ..<br>if all you need is a system without any printers then this is a serious regression.<br><br>I am not sure I am following why doCompare() is the place to fix this.<br>If getRemotePrinterNames() is returning NULL strings, then maybe it should not !<br><br>I suggest to fix it there.<br><br>-phil.<br><br><br><br><br><br><br><br><o:p></o:p></p><div><p class=MsoNormal>On 11/26/18 7:51 AM, Prasanta Sadhukhan wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p>I am not against doCompare() changes. I am just saying run() changes are not needed.<o:p></o:p></p><p class=MsoNormal>Regards<br>Prasanta<o:p></o:p></p><div><p class=MsoNormal>On 26-Nov-18 9:15 PM, Shashidhara Veerabhadraiah wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span style='color:#1F497D'>There is a possible case and that is the reason for this fix. The possible states for prevRemotePinters and currentRemotePrinters are null and valid values and they can reach those states at any time either because of network disconnect or any other OS related changes. With that in mind, this fix tries to eliminate the possible NPE cases.</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Thanks and regards,</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'>Shashi</span><o:p></o:p></p><p class=MsoNormal><span style='color:#1F497D'> </span><o:p></o:p></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='color:windowtext'>From:</span></b><span style='color:windowtext'> Prasanta Sadhukhan <br><b>Sent:</b> Monday, November 26, 2018 8:01 PM<br><b>To:</b> Shashidhara Veerabhadraiah <a href="mailto:shashidhara.veerabhadraiah@oracle.com"><shashidhara.veerabhadraiah@oracle.com></a>; <a href="mailto:swing-dev@openjdk.java.net">swing-dev@openjdk.java.net</a>; <a href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a><br><b>Subject:</b> Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732</span><o:p></o:p></p></div></div><p class=MsoNormal> <o:p></o:p></p><p><span style='font-size:12.0pt'> </span><o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><div><p class=MsoNormal>On 26-Nov-18 6:51 PM, <a href="mailto:shashidhara.veerabhadraiah@oracle.com">shashidhara.veerabhadraiah@oracle.com</a> wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p>Hi Prasanta, I think we should not create a behavior across the functions. doCompare() does only the comparison and it may be used for other purposes and is complete with respect to the comparison functionality.<o:p></o:p></p><p>run() function has a different behavior as it needs to populate the prevRemotePrinters and then the currentRemotePrinters and then use the comparison functionality. I think this is a good way to do.<o:p></o:p></p></blockquote><p class=MsoNormal style='margin-bottom:12.0pt'>Even without the if-else check, it does populates the prevRemotePrinters, no? <br>if "prevRemotePrinters" is null and currentRemotePrinters is null, then no need to update. If currentRemotePrinters is null, then what is the point of using getRemotePrintersNames() to update prevRemotePrinters as currentRemotePrinters is also obtained from getRemotePrintersNames!!<br>If "prevRemotePrinters" is null and currentRemotePrinters is not null, then doCompare() called from run() will be true and prevRemotePrinters will be populated with currentRemotePrinters.<br>If "prevRemotePrinters" is not null and currentRemotePrinters is null, then also prevRemotePrinters will be populated with currentRemotePrinters.<br><br>Regards<br>Prasanta<br><br><br><br><br><br><br><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p>Thanks and regards,<o:p></o:p></p><p>Shashi<o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><div><p class=MsoNormal>On 26/11/18 6:03 PM, Prasanta Sadhukhan wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p>Hi Shashi,<o:p></o:p></p><p class=MsoNormal>I think l437 check of if-else <span class=new><b><span style='font-size:10.0pt;font-family:Consolas;color:blue'>if (prevRemotePrinters != null) {</span></b></span><o:p></o:p></p><p class=MsoNormal><span style='font-size:10.0pt;font-family:Consolas'> </span><o:p></o:p></p><p class=MsoNormal style='margin-bottom:12.0pt'>is not required. prevRemotePrinters null check is addressed in str1==null case in doCompare().<br>If prevRemotePrinters is null and currentRemotePrinters is not null, then you update prevRemotePrinters to currentRemotePrinters as per l415 where doCompare returns true.<br>Also, If prevRemotePrinters is not null and currentRemotePrinters is null, then also you update prevRemotePrinters to currentRemotePrinters which is the output of getRemotePrintersNames().<br><br>Regards<br>Prasanta<br><br><br><br><br><br><br><br><o:p></o:p></p><div><p class=MsoNormal>On 26-Nov-18 2:33 PM, Shashidhara Veerabhadraiah wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal>Hi All, Please review a NPE fix for the below bug.<o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>Bug: <a href="https://bugs.openjdk.java.net/browse/JDK-8212202">https://bugs.openjdk.java.net/browse/JDK-8212202</a><o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>Webrev: <a href="http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.00/">http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.00/</a><o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>Function getRemotePrintersNames() may return null values and hence they need to be handled from the caller of that function which was missing earlier. This fix handles the null return values of the said function.<o:p></o:p></p><p class=MsoNormal> <o:p></o:p></p><p class=MsoNormal>Thanks and regards,<o:p></o:p></p><p class=MsoNormal>Shashi<o:p></o:p></p></blockquote><p class=MsoNormal><span style='font-size:12.0pt;font-family:"Times New Roman",serif'> </span><o:p></o:p></p></blockquote><p class=MsoNormal><span style='font-size:12.0pt;font-family:"Times New Roman",serif'> </span><o:p></o:p></p></blockquote><p class=MsoNormal><span style='font-size:12.0pt;font-family:"Times New Roman",serif'> </span><o:p></o:p></p></blockquote><p class=MsoNormal><span style='font-size:12.0pt;font-family:"Times New Roman",serif'> </span><o:p></o:p></p></blockquote><p class=MsoNormal><span style='font-size:12.0pt;font-family:"Times New Roman",serif'> </span><o:p></o:p></p></blockquote><p class=MsoNormal><span style='font-size:12.0pt;font-family:"Times New Roman",serif'> </span><o:p></o:p></p></blockquote><p class=MsoNormal><span style='font-size:12.0pt;font-family:"Times New Roman",serif'> </span><o:p></o:p></p></blockquote></blockquote><p class=MsoNormal><span style='font-size:12.0pt;font-family:"Times New Roman",serif'> </span><o:p></o:p></p></blockquote></blockquote><p class=MsoNormal><span style='font-size:12.0pt;font-family:"Times New Roman ,serif",serif'> </span><o:p></o:p></p></blockquote><p class=MsoNormal><span style='font-size:12.0pt;font-family:"Times New Roman ,serif",serif'> </span><o:p></o:p></p></blockquote><p class=MsoNormal><span style='font-size:12.0pt;font-family:"Times New Roman",serif'><o:p> </o:p></span></p></div></body></html>