[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