[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>; swing-...@openjdk.java.net;
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