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.veerabhadra...@oracle.com>; Prasanta Sadhukhan
<prasanta.sadhuk...@oracle.com>; 2d-dev@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.sadhuk...@oracle.com>
<mailto:prasanta.sadhuk...@oracle.com>; Shashidhara Veerabhadraiah
<shashidhara.veerabhadra...@oracle.com>
<mailto:shashidhara.veerabhadra...@oracle.com>;
2d-dev@openjdk.java.net <mailto:2d-dev@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.veerabhadra...@oracle.com>
<mailto:shashidhara.veerabhadra...@oracle.com>;
swing-...@openjdk.java.net
<mailto:swing-...@openjdk.java.net>;
2d-dev@openjdk.java.net <mailto:2d-dev@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.veerabhadra...@oracle.com
<mailto:shashidhara.veerabhadra...@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
