[OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732

Phil Race philip.race at oracle.com
Wed Dec 5 21:05:07 UTC 2018


But what I am saying is we should not return a NULL string reference
from native. You are still allowing that and then having to handle
it in Java code.

And FWIW you can return a zero length array as well so there
isn't a NULL array reference to deal with either.

-phil.

On 12/3/18 8:29 AM, Shashidhara Veerabhadraiah wrote:
>
> 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:
>
> http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.02/
>
> 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.
>
> Thanks and regards,
>
> Shashi
>
> *From:*Phil Race
> *Sent:* Wednesday, November 28, 2018 11:19 PM
> *To:* Shashidhara Veerabhadraiah 
> <shashidhara.veerabhadraiah at oracle.com>; Prasanta Sadhukhan 
> <prasanta.sadhukhan at oracle.com>; 2d-dev at openjdk.java.net
> *Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print 
> tests after JDK-8153732
>
> I am not understanding you. I thought the problem to be we got an 
> array of (say) 3 values
> (ie printer names) returned from native where some or all of the 
> *values* were NULL.
> And I am saying we should in such a case in the native code, before 
> returning,
> remove from the returned array all values that are NULL.
> That could result in an empty (zero length) array being returned from 
> native but
> no null names ..
>
> -phil.
>
> On 11/26/18 10:23 PM, Shashidhara Veerabhadraiah wrote:
>
>     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.
>
>     Here is the updated Webrev for test fix:
>
>     http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.01/
>
>     Thanks and regards,
>     Shashi
>
>     *From:*Phil Race
>     *Sent:* Tuesday, November 27, 2018 1:52 AM
>     *To:* Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com>
>     <mailto:prasanta.sadhukhan at oracle.com>; Shashidhara Veerabhadraiah
>     <shashidhara.veerabhadraiah at oracle.com>
>     <mailto:shashidhara.veerabhadraiah at oracle.com>;
>     2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>
>     *Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print
>     tests after JDK-8153732
>
>     [Removed swing-dev as this as nothing to do with swing].
>
>     You mention in the bug eval that you don't need a new test because
>     this
>     is already covered by the test for 8153732. If that is the case
>     then this
>     bugid should be added to that test.
>     Although it also looks like there are plenty of tests that provoke
>     this ..
>     if all you need is a system without any printers then this is a
>     serious regression.
>
>     I am not sure I am following why doCompare() is the place to fix this.
>     If getRemotePrinterNames() is returning NULL strings, then maybe
>     it should not !
>
>     I suggest to fix it there.
>
>     -phil.
>
>
>
>     On 11/26/18 7:51 AM, Prasanta Sadhukhan wrote:
>
>         I am not against doCompare() changes. I am just saying run()
>         changes are not needed.
>
>         Regards
>         Prasanta
>
>         On 26-Nov-18 9:15 PM, Shashidhara Veerabhadraiah wrote:
>
>             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.
>
>             Thanks and regards,
>
>             Shashi
>
>             *From:*Prasanta Sadhukhan
>             *Sent:* Monday, November 26, 2018 8:01 PM
>             *To:* Shashidhara Veerabhadraiah
>             <shashidhara.veerabhadraiah at oracle.com>
>             <mailto:shashidhara.veerabhadraiah at oracle.com>;
>             swing-dev at openjdk.java.net
>             <mailto:swing-dev at openjdk.java.net>;
>             2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>
>             *Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in
>             the print tests after JDK-8153732
>
>             On 26-Nov-18 6:51 PM,
>             shashidhara.veerabhadraiah at oracle.com
>             <mailto:shashidhara.veerabhadraiah at oracle.com> wrote:
>
>                 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.
>
>                 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.
>
>             Even without the if-else check, it does populates the
>             prevRemotePrinters, no?
>             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!!
>             If "prevRemotePrinters" is null and currentRemotePrinters
>             is not null, then doCompare() called from run() will be
>             true and prevRemotePrinters will be populated with
>             currentRemotePrinters.
>             If "prevRemotePrinters" is not null and
>             currentRemotePrinters is null, then also
>             prevRemotePrinters will be populated with
>             currentRemotePrinters.
>
>             Regards
>             Prasanta
>
>
>
>                 Thanks and regards,
>
>                 Shashi
>
>                 On 26/11/18 6:03 PM, Prasanta Sadhukhan wrote:
>
>                     Hi Shashi,
>
>                     I think l437 check of if-else *if
>                     (prevRemotePrinters != null) {*
>
>                     is not required. prevRemotePrinters null check is
>                     addressed in str1==null case in doCompare().
>                     If prevRemotePrinters is null and
>                     currentRemotePrinters is not null, then you update
>                     prevRemotePrinters to currentRemotePrinters as per
>                     l415 where doCompare returns true.
>                     Also, If prevRemotePrinters is not null and
>                     currentRemotePrinters is null, then also you
>                     update prevRemotePrinters to currentRemotePrinters
>                     which is the output of getRemotePrintersNames().
>
>                     Regards
>                     Prasanta
>
>
>
>                     On 26-Nov-18 2:33 PM, Shashidhara Veerabhadraiah
>                     wrote:
>
>                         Hi All, Please review a NPE fix for the below bug.
>
>                         Bug:
>                         https://bugs.openjdk.java.net/browse/JDK-8212202
>
>                         Webrev:
>                         http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.00/
>                         <http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.00/>
>
>                         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.
>
>                         Thanks and regards,
>
>                         Shashi
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20181205/5256c00f/attachment-0001.html>


More information about the 2d-dev mailing list