Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Thank you Phil for clearing this. Thank you Prasanta for your review. Thanks and regards, Shashi From: Philip Race Sent: Sunday, February 24, 2019 4:43 AM To: Shashidhara Veerabhadraiah Cc: Prasanta Sadhukhan ; 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 I think the code as it is, is correct and safe. The proposed optimisation would have very little measurable benefit for something that creates such a dependency on knowing the implementation of some other piece of code. So we should push it as it is now. -phil. On 2/19/19, 9:15 PM, Shashidhara Veerabhadraiah wrote: Thank you Prashanta. This bug(NPE) is already a regression for an earlier bug and do not wish to cause one more in future for a poorly creation of the function definition especially when it is not implied by the function definition. That's my intent. The null check expression should not even take a single cpu cycle per me and running it in 4 mins is definitely fine I feel rather than creating a path for a future NPE(it's very easy to overlook per me when not implied). So any other volunteers for reviewing this bug. Thanks and regards, Shashi From: Prasanta Sadhukhan Sent: Monday, February 18, 2019 5:39 PM To: Shashidhara Veerabhadraiah HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com;; Philip Race HYPERLINK "mailto:philip.r...@oracle.com; Cc: HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 Hi Shashi, I think if the overhead can be fixed with a small change, we should do it as I think we need to keep the code optimized with whatever code is present now. If in future, if somebody changes the function order, like doCompare(prev..., curr..) to doCompare(curr..., prev..) [what you are trying to imply] then it needs to be done with proper reasoning (which I am not sure what it can be and then we can do these checks). That's my take. It other reviewers feel the present changes are ok, you can commit the changes with their approval. Regards Prasanta On 18-Feb-19 12:26 PM, Shashidhara Veerabhadraiah wrote: Hi Prasanta, I hope I have answered your questions satisfactorily. Please let me know if you have any more questions. Thanks and regards, Shashi From: Shashidhara Veerabhadraiah Sent: Friday, February 15, 2019 12:37 PM To: Prasanta Sadhukhan HYPERLINK "mailto:prasanta.sadhuk...@oracle.com;; Philip Race HYPERLINK "mailto:philip.r...@oracle.com; Cc: HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net Subject: RE: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 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. Thanks and regards, Shashi From: Prasanta Sadhukhan Sent: Friday, February 15, 2019 12:28 PM To: Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com>; Philip Race mailto:philip.r...@oracle.com"philip.r...@oracle.com> Cc: HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 Hi Shashi, 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. 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. Regards Prasanta On 15-Feb-19 12:17 PM, Shashidhara Veerabhadraiah wrote: 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: doCompare(currentRemotePrinters, prevRemotePrinters). 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. Hope you agree with me. Thanks and regards, Sh
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi Shashi,
I think if the overhead can be fixed with a small change, we should do
it as I think we need to keep the code optimized with whatever code is
present now.
If in future, if somebody changes the function order, like
doCompare(prev..., curr..) to doCompare(curr..., prev..) [what you are
trying to imply]
then it needs to be done with proper reasoning (which I am not sure what
it can be and then we can do these checks). That's my take. It other
reviewers feel the present changes are ok, you can commit the changes
with their approval.
Regards
Prasanta
On 18-Feb-19 12:26 PM, Shashidhara Veerabhadraiah wrote:
Hi Prasanta, I hope I have answered your questions satisfactorily.
Please let me know if you have any more questions.
Thanks and regards,
Shashi
*From:*Shashidhara Veerabhadraiah
*Sent:* Friday, February 15, 2019 12:37 PM
*To:* Prasanta Sadhukhan ; Philip Race
*Cc:* 2d-dev@openjdk.java.net
*Subject:* RE: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print
tests after JDK-8153732
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.
Thanks and regards,
Shashi
*From:*Prasanta Sadhukhan
*Sent:* Friday, February 15, 2019 12:28 PM
*To:* Shashidhara Veerabhadraiah
<mailto:shashidhara.veerabhadra...@oracle.com>>; Philip Race
mailto:philip.r...@oracle.com>>
*Cc:* 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
Hi Shashi,
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.
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.
Regards
Prasanta
On 15-Feb-19 12:17 PM, Shashidhara Veerabhadraiah wrote:
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:
doCompare(currentRemotePrinters, prevRemotePrinters). 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.
Hope you agree with me.
Thanks and regards,
Shashi
*From:*Prasanta Sadhukhan
*Sent:* Friday, February 15, 2019 12:11 PM
*To:* Phil Race
<mailto:philip.r...@oracle.com>; Shashidhara Veerabhadraiah
<mailto:shashidhara.veerabhadra...@oracle.com>
*Cc:* 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
Hi Shashi,
I have one comment:
doCompare(prevRemotePrinters, currentRemotePrinters) is only
called from run() method when "prevRemotePrinters" is already
checked to be not null [*if (prevRemotePrinters != null &&
prevRemotePrinters.length > 0) ]*
so I see no point in checking "str1" [which is prevRemotePrinters]
to be null in doCompare(). I guess instead of
*413 if (str1 == null && str2 == null) {*
*414 return false;*
*415 } else if (str1 == null || str2 == null) {*
*416 return true;*
*417 }*
you can have
if (str2 == null)
return true;
Regards
Prasanta
On 15-Feb-19 2:48 AM, Phil Race wrote:
+1 .. remember to use the current bug synopsis in the push comment
ie : "[Windows] Exception if no printers are installed."
-phil.
On 2/11/19 12:39 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the new Webrev fixing those comments:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.06/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.06/>
Thanks and regards,
Shashi
*From:*Philip Race
*Sent:* Saturday, February 9, 2019 2:25 AM
*To:* Shashidhara Veerabhadraiah
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi Prasanta, I hope I have answered your questions satisfactorily. Please let
me know if you have any more questions.
Thanks and regards,
Shashi
From: Shashidhara Veerabhadraiah
Sent: Friday, February 15, 2019 12:37 PM
To: Prasanta Sadhukhan ; Philip Race
Cc: 2d-dev@openjdk.java.net
Subject: RE: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
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.
Thanks and regards,
Shashi
From: Prasanta Sadhukhan
Sent: Friday, February 15, 2019 12:28 PM
To: Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com>;
Philip Race mailto:philip.r...@oracle.com"philip.r...@oracle.com>
Cc: HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Hi Shashi,
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.
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.
Regards
Prasanta
On 15-Feb-19 12:17 PM, Shashidhara Veerabhadraiah wrote:
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: doCompare(currentRemotePrinters,
prevRemotePrinters). 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.
Hope you agree with me.
Thanks and regards,
Shashi
From: Prasanta Sadhukhan
Sent: Friday, February 15, 2019 12:11 PM
To: Phil Race HYPERLINK
"mailto:philip.r...@oracle.com;; Shashidhara
Veerabhadraiah HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com;
Cc: HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Hi Shashi,
I have one comment:
doCompare(prevRemotePrinters, currentRemotePrinters) is only called from run()
method when "prevRemotePrinters" is already checked to be not null [if
(prevRemotePrinters != null && prevRemotePrinters.length > 0) ]
so I see no point in checking "str1" [which is prevRemotePrinters] to be null
in doCompare(). I guess instead of
413 if (str1 == null && str2 == null) {
414 return false;
415 } else if (str1 == null || str2 == null) {
416 return true;
417 }
you can have
if (str2 == null)
return true;
Regards
Prasanta
On 15-Feb-19 2:48 AM, Phil Race wrote:
+1 .. remember to use the current bug synopsis in the push comment
ie : "[Windows] Exception if no printers are installed."
-phil.
On 2/11/19 12:39 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the new Webrev fixing those comments:
HYPERLINK
"http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.06/"http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.06/
Thanks and regards,
Shashi
From: Philip Race
Sent: Saturday, February 9, 2019 2:25 AM
To: Shashidhara Veerabhadraiah HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com;
Cc: Prasanta Sadhukhan HYPERLINK
"mailto:prasanta.sadhuk...@oracle.com;;
HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
413 if (str1 == null && str2 == null) {
414 return false;
415 } else if ((str1 == null && str2 != null) || (str2 == null &&
str1 != null)) {
416 return true;
417 }
When we get to 415 we already know at least one of str1 or str2 is non-null so
415 can just be
} else if (str1 == null || str2 == null) {
-phil.
On 2/6/19, 12:31 AM, Shashidha
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
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.
Thanks and regards,
Shashi
From: Prasanta Sadhukhan
Sent: Friday, February 15, 2019 12:28 PM
To: Shashidhara Veerabhadraiah ; Philip
Race
Cc: 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Hi Shashi,
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.
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.
Regards
Prasanta
On 15-Feb-19 12:17 PM, Shashidhara Veerabhadraiah wrote:
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: doCompare(currentRemotePrinters,
prevRemotePrinters). 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.
Hope you agree with me.
Thanks and regards,
Shashi
From: Prasanta Sadhukhan
Sent: Friday, February 15, 2019 12:11 PM
To: Phil Race HYPERLINK
"mailto:philip.r...@oracle.com;; Shashidhara
Veerabhadraiah HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com;
Cc: HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Hi Shashi,
I have one comment:
doCompare(prevRemotePrinters, currentRemotePrinters) is only called from run()
method when "prevRemotePrinters" is already checked to be not null [if
(prevRemotePrinters != null && prevRemotePrinters.length > 0) ]
so I see no point in checking "str1" [which is prevRemotePrinters] to be null
in doCompare(). I guess instead of
413 if (str1 == null && str2 == null) {
414 return false;
415 } else if (str1 == null || str2 == null) {
416 return true;
417 }
you can have
if (str2 == null)
return true;
Regards
Prasanta
On 15-Feb-19 2:48 AM, Phil Race wrote:
+1 .. remember to use the current bug synopsis in the push comment
ie : "[Windows] Exception if no printers are installed."
-phil.
On 2/11/19 12:39 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the new Webrev fixing those comments:
HYPERLINK
"http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.06/"http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.06/
Thanks and regards,
Shashi
From: Philip Race
Sent: Saturday, February 9, 2019 2:25 AM
To: Shashidhara Veerabhadraiah HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com;
Cc: Prasanta Sadhukhan HYPERLINK
"mailto:prasanta.sadhuk...@oracle.com;;
HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
413 if (str1 == null && str2 == null) {
414 return false;
415 } else if ((str1 == null && str2 != null) || (str2 == null &&
str1 != null)) {
416 return true;
417 }
When we get to 415 we already know at least one of str1 or str2 is non-null so
415 can just be
} else if (str1 == null || str2 == null) {
-phil.
On 2/6/19, 12:31 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the updated Webrev fixing those comments:
HYPERLINK
"http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.05/"http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.05/
Thanks and regards,
Shashi
From: Phil Race
Sent: Thursday, January 31, 2019 4:36 AM
To: Shashidhara Veerabhadraiah HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com;
Cc: Prasanta Sadhukhan HYPERLINK
"mailto:prasanta.sadhuk...@oracle.com;;
HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi Shashi,
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.
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.
Regards
Prasanta
On 15-Feb-19 12:17 PM, Shashidhara Veerabhadraiah wrote:
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:
doCompare(currentRemotePrinters, prevRemotePrinters). 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.
Hope you agree with me.
Thanks and regards,
Shashi
*From:*Prasanta Sadhukhan
*Sent:* Friday, February 15, 2019 12:11 PM
*To:* Phil Race ; Shashidhara Veerabhadraiah
*Cc:* 2d-dev@openjdk.java.net
*Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print
tests after JDK-8153732
Hi Shashi,
I have one comment:
doCompare(prevRemotePrinters, currentRemotePrinters) is only called
from run() method when "prevRemotePrinters" is already checked to be
not null [*if (prevRemotePrinters != null && prevRemotePrinters.length
> 0) ]*
so I see no point in checking "str1" [which is prevRemotePrinters] to
be null in doCompare(). I guess instead of
*413 if (str1 == null && str2 == null) {*
*414 return false;*
*415 } else if (str1 == null || str2 == null) {*
*416 return true;*
*417 }*
you can have
if (str2 == null)
return true;
Regards
Prasanta
On 15-Feb-19 2:48 AM, Phil Race wrote:
+1 .. remember to use the current bug synopsis in the push comment
ie : "[Windows] Exception if no printers are installed."
-phil.
On 2/11/19 12:39 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the new Webrev fixing those comments:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.06/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.06/>
Thanks and regards,
Shashi
*From:*Philip Race
*Sent:* Saturday, February 9, 2019 2:25 AM
*To:* Shashidhara Veerabhadraiah
<mailto:shashidhara.veerabhadra...@oracle.com>
*Cc:* Prasanta Sadhukhan
<mailto:prasanta.sadhuk...@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
413 if (str1 == null && str2 == null) {
414 return false;
415 } else if ((str1 == null && str2 != null) ||
(str2 == null && str1 != null)) {
416 return true;
417 }
When we get to 415 we already know at least one of str1 or
str2 is non-null so 415 can just be
} else if (str1 == null || str2 == null) {
-phil.
On 2/6/19, 12:31 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the updated Webrev fixing those comments:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.05/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.05/>
Thanks and regards,
Shashi
*From:*Phil Race
*Sent:* Thursday, January 31, 2019 4:36 AM
*To:* Shashidhara Veerabhadraiah
<mailto:shashidhara.veerabhadra...@oracle.com>
*Cc:* Prasanta Sadhukhan
<mailto:prasanta.sadhuk...@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
Sorry .. this got lost ..
I don't understand this line :
253 jobjectArray emptyArray = env->NewObjectArray(1,
clazz, NULL);
This is returning an array of length 1 and element 0 is NULL.
I think you want
env->NewObjectArray(0, clazz, NULL);
and I don't see why you need to create it there
instead you can just have
304 if (nameArray != NULL) {
305 return nameArray;
306 } else {
307
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
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: doCompare(currentRemotePrinters,
prevRemotePrinters). 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.
Hope you agree with me.
Thanks and regards,
Shashi
From: Prasanta Sadhukhan
Sent: Friday, February 15, 2019 12:11 PM
To: Phil Race ; Shashidhara Veerabhadraiah
Cc: 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Hi Shashi,
I have one comment:
doCompare(prevRemotePrinters, currentRemotePrinters) is only called from run()
method when "prevRemotePrinters" is already checked to be not null [if
(prevRemotePrinters != null && prevRemotePrinters.length > 0) ]
so I see no point in checking "str1" [which is prevRemotePrinters] to be null
in doCompare(). I guess instead of
413 if (str1 == null && str2 == null) {
414 return false;
415 } else if (str1 == null || str2 == null) {
416 return true;
417 }
you can have
if (str2 == null)
return true;
Regards
Prasanta
On 15-Feb-19 2:48 AM, Phil Race wrote:
+1 .. remember to use the current bug synopsis in the push comment
ie : "[Windows] Exception if no printers are installed."
-phil.
On 2/11/19 12:39 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the new Webrev fixing those comments:
HYPERLINK
"http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.06/"http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.06/
Thanks and regards,
Shashi
From: Philip Race
Sent: Saturday, February 9, 2019 2:25 AM
To: Shashidhara Veerabhadraiah HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com;
Cc: Prasanta Sadhukhan HYPERLINK
"mailto:prasanta.sadhuk...@oracle.com;;
HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
413 if (str1 == null && str2 == null) {
414 return false;
415 } else if ((str1 == null && str2 != null) || (str2 == null &&
str1 != null)) {
416 return true;
417 }
When we get to 415 we already know at least one of str1 or str2 is non-null so
415 can just be
} else if (str1 == null || str2 == null) {
-phil.
On 2/6/19, 12:31 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the updated Webrev fixing those comments:
HYPERLINK
"http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.05/"http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.05/
Thanks and regards,
Shashi
From: Phil Race
Sent: Thursday, January 31, 2019 4:36 AM
To: Shashidhara Veerabhadraiah HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com;
Cc: Prasanta Sadhukhan HYPERLINK
"mailto:prasanta.sadhuk...@oracle.com;;
HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Sorry .. this got lost ..
I don't understand this line :
253 jobjectArray emptyArray = env->NewObjectArray(1, clazz, NULL);
This is returning an array of length 1 and element 0 is NULL.
I think you want
env->NewObjectArray(0, clazz, NULL);
and I don't see why you need to create it there
instead you can just have
304 if (nameArray != NULL) {
305 return nameArray;
306 } else {
307 return env->NewObjectArray(0, clazz, NULL);
308 }
449 if (prevRemotePrinters != null) {
shouldn't this be
449 if (prevRemotePrinters != null &&
prevRemotePrinters.length > 0) {
?
-phil.
On 12/10/18 10:19 PM, Shashidhara Veerabhadraiah wrote:
Hi Phil, I have updated with small code updates:
HYPERLINK
"http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.04/"http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.04/
Thanks and regards,
Shashi
From: Shashidhara Veerabhadraiah
Sent: Monday, December 10, 2018 3:19 PM
To: Philip Race HYPERLINK
"mailto:philip.r...@oracle.com;
Cc: Prasanta Sadhukhan HYPERLINK
"mailto:prasanta.sadhuk...@oracle.com;;
HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: RE: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Hi Phil, Please find the updated Webrev.
HYPERLINK
"http://cr.openjdk.java.n
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi Shashi,
I have one comment:
doCompare(prevRemotePrinters, currentRemotePrinters) is only called from
run() method when "prevRemotePrinters" is already checked to be not null
[if (prevRemotePrinters != null && prevRemotePrinters.length > 0) ]
so I see no point in checking "str1" [which is prevRemotePrinters] to be
null in doCompare(). I guess instead of
413 if (str1 == null && str2 == null) {414 return false;415 } else if
(str1 == null || str2 == null) {416 return true;417 }you can have if
(str2 == null) return true; Regards Prasanta
On 15-Feb-19 2:48 AM, Phil Race wrote:
+1 .. remember to use the current bug synopsis in the push comment
ie : "[Windows] Exception if no printers are installed."
-phil.
On 2/11/19 12:39 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the new Webrev fixing those comments:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.06/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.06/>
Thanks and regards,
Shashi
*From:*Philip Race
*Sent:* Saturday, February 9, 2019 2:25 AM
*To:* Shashidhara Veerabhadraiah
*Cc:* Prasanta Sadhukhan ;
2d-dev@openjdk.java.net
*Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print
tests after JDK-8153732
413 if (str1 == null && str2 == null) {
414 return false;
415 } else if ((str1 == null && str2 != null) || (str2
== null && str1 != null)) {
416 return true;
417 }
When we get to 415 we already know at least one of str1 or str2 is
non-null so 415 can just be
} else if (str1 == null || str2 == null) {
-phil.
On 2/6/19, 12:31 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the updated Webrev fixing those comments:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.05/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.05/>
Thanks and regards,
Shashi
*From:*Phil Race
*Sent:* Thursday, January 31, 2019 4:36 AM
*To:* Shashidhara Veerabhadraiah
<mailto:shashidhara.veerabhadra...@oracle.com>
*Cc:* Prasanta Sadhukhan
<mailto:prasanta.sadhuk...@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
Sorry .. this got lost ..
I don't understand this line :
253 jobjectArray emptyArray = env->NewObjectArray(1, clazz,
NULL);
This is returning an array of length 1 and element 0 is NULL.
I think you want
env->NewObjectArray(0, clazz, NULL);
and I don't see why you need to create it there
instead you can just have
304 if (nameArray != NULL) {
305 return nameArray;
306 } else {
307 return env->NewObjectArray(0, clazz, NULL);
308 }
449 if (prevRemotePrinters != null) {
shouldn't this be
449 if (prevRemotePrinters != null &&
prevRemotePrinters.length > 0) {
?
-phil.
On 12/10/18 10:19 PM, Shashidhara Veerabhadraiah wrote:
Hi Phil, I have updated with small code updates:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.04/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.04/>
Thanks and regards,
Shashi
*From:*Shashidhara Veerabhadraiah
*Sent:* Monday, December 10, 2018 3:19 PM
*To:* Philip Race
<mailto:philip.r...@oracle.com>
*Cc:* Prasanta Sadhukhan
<mailto:prasanta.sadhuk...@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
Hi Phil, Please find the updated Webrev.
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.03/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.03/>
Thanks and regards,
Shashi
*From:*Philip Race
*Sent:* Friday, December 7, 2018 8:54 PM
*To:* Shashidhara Veerabhadraiah
mailto:shashidhara.veerabhadra...@oracle.com>>
*Cc:* Prasanta Sadhukhan mailto:prasanta.sadhuk...@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
First off, I think the high order bit is that if a non-null
array reference is returned there are NO
null String entries in it. Whether a zero length or null
array is returned when there are no printers
is the less important issue.
However an empty array also tells you there are no printers,
and you are less likely to get an NPE from that.
It i
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
+1 .. remember to use the current bug synopsis in the push comment
ie : "[Windows] Exception if no printers are installed."
-phil.
On 2/11/19 12:39 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the new Webrev fixing those comments:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.06/
Thanks and regards,
Shashi
*From:*Philip Race
*Sent:* Saturday, February 9, 2019 2:25 AM
*To:* Shashidhara Veerabhadraiah
*Cc:* Prasanta Sadhukhan ;
2d-dev@openjdk.java.net
*Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print
tests after JDK-8153732
413 if (str1 == null && str2 == null) {
414 return false;
415 } else if ((str1 == null && str2 != null) || (str2 ==
null && str1 != null)) {
416 return true;
417 }
When we get to 415 we already know at least one of str1 or str2 is
non-null so 415 can just be
} else if (str1 == null || str2 == null) {
-phil.
On 2/6/19, 12:31 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the updated Webrev fixing those comments:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.05/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.05/>
Thanks and regards,
Shashi
*From:*Phil Race
*Sent:* Thursday, January 31, 2019 4:36 AM
*To:* Shashidhara Veerabhadraiah
<mailto:shashidhara.veerabhadra...@oracle.com>
*Cc:* Prasanta Sadhukhan
<mailto:prasanta.sadhuk...@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
Sorry .. this got lost ..
I don't understand this line :
253 jobjectArray emptyArray = env->NewObjectArray(1, clazz, NULL);
This is returning an array of length 1 and element 0 is NULL.
I think you want
env->NewObjectArray(0, clazz, NULL);
and I don't see why you need to create it there
instead you can just have
304 if (nameArray != NULL) {
305 return nameArray;
306 } else {
307 return env->NewObjectArray(0, clazz, NULL);
308 }
449 if (prevRemotePrinters != null) {
shouldn't this be
449 if (prevRemotePrinters != null &&
prevRemotePrinters.length > 0) {
?
-phil.
On 12/10/18 10:19 PM, Shashidhara Veerabhadraiah wrote:
Hi Phil, I have updated with small code updates:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.04/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.04/>
Thanks and regards,
Shashi
*From:*Shashidhara Veerabhadraiah
*Sent:* Monday, December 10, 2018 3:19 PM
*To:* Philip Race
<mailto:philip.r...@oracle.com>
*Cc:* Prasanta Sadhukhan
<mailto:prasanta.sadhuk...@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
Hi Phil, Please find the updated Webrev.
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.03/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.03/>
Thanks and regards,
Shashi
*From:*Philip Race
*Sent:* Friday, December 7, 2018 8:54 PM
*To:* Shashidhara Veerabhadraiah
mailto:shashidhara.veerabhadra...@oracle.com>>
*Cc:* Prasanta Sadhukhan mailto:prasanta.sadhuk...@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
First off, I think the high order bit is that if a non-null
array reference is returned there are NO
null String entries in it. Whether a zero length or null array
is returned when there are no printers
is the less important issue.
However an empty array also tells you there are no printers,
and you are less likely to get an NPE from that.
It is arguably easier for the caller, if they don't need the
extra null check first.
FWIW the javax.print public APIs return zero length arrays
instead of null and applications seem to survive :-)
I don't know what you mean by :
> (And anyway we need to handle the first null string reference)?
If you are referring to a small matter of coding in the native
layer, then that is not an insurmountable problem.
Basically if there are problems getting names or whatever in
the native layer, handle it THERE, don't make
everyone else have to deal with it.
One caveat: JNI calls can theoretically fail due to OOME ..
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi Phil, Here is the new Webrev fixing those comments:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.06/
Thanks and regards,
Shashi
From: Philip Race
Sent: Saturday, February 9, 2019 2:25 AM
To: Shashidhara Veerabhadraiah
Cc: Prasanta Sadhukhan ; 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
413 if (str1 == null && str2 == null) {
414 return false;
415 } else if ((str1 == null && str2 != null) || (str2 == null &&
str1 != null)) {
416 return true;
417 }
When we get to 415 we already know at least one of str1 or str2 is non-null so
415 can just be
} else if (str1 == null || str2 == null) {
-phil.
On 2/6/19, 12:31 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the updated Webrev fixing those comments:
HYPERLINK
"http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.05/"http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.05/
Thanks and regards,
Shashi
From: Phil Race
Sent: Thursday, January 31, 2019 4:36 AM
To: Shashidhara Veerabhadraiah HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com;
Cc: Prasanta Sadhukhan HYPERLINK
"mailto:prasanta.sadhuk...@oracle.com;;
HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Sorry .. this got lost ..
I don't understand this line :
253 jobjectArray emptyArray = env->NewObjectArray(1, clazz, NULL);
This is returning an array of length 1 and element 0 is NULL.
I think you want
env->NewObjectArray(0, clazz, NULL);
and I don't see why you need to create it there
instead you can just have
304 if (nameArray != NULL) {
305 return nameArray;
306 } else {
307 return env->NewObjectArray(0, clazz, NULL);
308 }
449 if (prevRemotePrinters != null) {
shouldn't this be
449 if (prevRemotePrinters != null &&
prevRemotePrinters.length > 0) {
?
-phil.
On 12/10/18 10:19 PM, Shashidhara Veerabhadraiah wrote:
Hi Phil, I have updated with small code updates:
HYPERLINK
"http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.04/"http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.04/
Thanks and regards,
Shashi
From: Shashidhara Veerabhadraiah
Sent: Monday, December 10, 2018 3:19 PM
To: Philip Race HYPERLINK
"mailto:philip.r...@oracle.com;
Cc: Prasanta Sadhukhan HYPERLINK
"mailto:prasanta.sadhuk...@oracle.com;;
HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: RE: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Hi Phil, Please find the updated Webrev.
HYPERLINK
"http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.03/"http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.03/
Thanks and regards,
Shashi
From: Philip Race
Sent: Friday, December 7, 2018 8:54 PM
To: Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com>
Cc: Prasanta Sadhukhan mailto:prasanta.sadhuk...@oracle.com"prasanta.sadhuk...@oracle.com>; HYPERLINK
"mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
First off, I think the high order bit is that if a non-null array reference is
returned there are NO
null String entries in it. Whether a zero length or null array is returned when
there are no printers
is the less important issue.
However an empty array also tells you there are no printers, and you are less
likely to get an NPE from that.
It is arguably easier for the caller, if they don't need the extra null check
first.
FWIW the javax.print public APIs return zero length arrays instead of null and
applications seem to survive :-)
I don't know what you mean by :
> (And anyway we need to handle the first null string reference)?
If you are referring to a small matter of coding in the native layer, then that
is not an insurmountable problem.
Basically if there are problems getting names or whatever in the native layer,
handle it THERE, don't make
everyone else have to deal with it.
One caveat: JNI calls can theoretically fail due to OOME .. in such a case we
are doomed and
the main goal is to not crash in native and to obey all JNI rules. What is
returned in that case
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.
-phil
On 12/6/18, 8:10 PM, Shashidhara Veerabhadraiah wrote:
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 shou
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
413 if (str1 == null && str2 == null) {
414 return false;
415 } else if ((str1 == null && str2 != null) || (str2 ==
null && str1 != null)) {
416 return true;
417 }
When we get to 415 we already know at least one of str1 or str2 is
non-null so 415 can just be
} else if (str1 == null || str2 == null) {
-phil.
On 2/6/19, 12:31 AM, Shashidhara Veerabhadraiah wrote:
Hi Phil, Here is the updated Webrev fixing those comments:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.05/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.05/>
Thanks and regards,
Shashi
*From:*Phil Race
*Sent:* Thursday, January 31, 2019 4:36 AM
*To:* Shashidhara Veerabhadraiah
*Cc:* Prasanta Sadhukhan ;
2d-dev@openjdk.java.net
*Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print
tests after JDK-8153732
Sorry .. this got lost ..
I don't understand this line :
253 jobjectArray emptyArray = env->NewObjectArray(1, clazz, NULL);
This is returning an array of length 1 and element 0 is NULL.
I think you want
env->NewObjectArray(0, clazz, NULL);
and I don't see why you need to create it there
instead you can just have
304 if (nameArray != NULL) {
305 return nameArray;
306 } else {
307 returnenv->NewObjectArray(0, clazz, NULL);
308 }
449 if (prevRemotePrinters != null) {
shouldn't this be
449 if (prevRemotePrinters != null&&
prevRemotePrinters.length> 0) {
?
-phil.
On 12/10/18 10:19 PM, Shashidhara Veerabhadraiah wrote:
Hi Phil, I have updated with small code updates:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.04/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.04/>
Thanks and regards,
Shashi
*From:*Shashidhara Veerabhadraiah
*Sent:* Monday, December 10, 2018 3:19 PM
*To:* Philip Race
<mailto:philip.r...@oracle.com>
*Cc:* Prasanta Sadhukhan
<mailto:prasanta.sadhuk...@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
Hi Phil, Please find the updated Webrev.
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.03/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.03/>
Thanks and regards,
Shashi
*From:*Philip Race
*Sent:* Friday, December 7, 2018 8:54 PM
*To:* Shashidhara Veerabhadraiah
mailto:shashidhara.veerabhadra...@oracle.com>>
*Cc:* Prasanta Sadhukhan mailto:prasanta.sadhuk...@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
First off, I think the high order bit is that if a non-null array
reference is returned there are NO
null String entries in it. Whether a zero length or null array is
returned when there are no printers
is the less important issue.
However an empty array also tells you there are no printers, and
you are less likely to get an NPE from that.
It is arguably easier for the caller, if they don't need the extra
null check first.
FWIW the javax.print public APIs return zero length arrays instead
of null and applications seem to survive :-)
I don't know what you mean by :
> (And anyway we need to handle the first null string reference)?
If you are referring to a small matter of coding in the native
layer, then that is not an insurmountable problem.
Basically if there are problems getting names or whatever in the
native layer, handle it THERE, don't make
everyone else have to deal with it.
One caveat: JNI calls can theoretically fail due to OOME .. in
such a case we are doomed and
the main goal is to not crash in native and to obey all JNI rules.
What is returned in that case
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.
-phil
On 12/6/18, 8:10 PM, Shashidhara Veerabhadraiah wrote:
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?
Thanks and regards,
Shashi
*From:*Phil Race
*Sent:* Thursday, December 6, 2018 2:35 AM
*To:* Shashidhara Veerabhadraiah
<mailto:shashidhara.veerabhadra...@oracle.com>; Prasanta
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi Phil, Here is the updated Webrev fixing those comments:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.05/
Thanks and regards,
Shashi
From: Phil Race
Sent: Thursday, January 31, 2019 4:36 AM
To: Shashidhara Veerabhadraiah
Cc: Prasanta Sadhukhan ; 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Sorry .. this got lost ..
I don't understand this line :
253 jobjectArray emptyArray = env->NewObjectArray(1, clazz, NULL);
This is returning an array of length 1 and element 0 is NULL.
I think you want
env->NewObjectArray(0, clazz, NULL);
and I don't see why you need to create it there
instead you can just have
304 if (nameArray != NULL) {
305 return nameArray;
306 } else {
307 return env->NewObjectArray(0, clazz, NULL);
308 }
449 if (prevRemotePrinters != null) {
shouldn't this be
449 if (prevRemotePrinters != null &&
prevRemotePrinters.length > 0) {
?
-phil.
On 12/10/18 10:19 PM, Shashidhara Veerabhadraiah wrote:
Hi Phil, I have updated with small code updates:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.04/
Thanks and regards,
Shashi
From: Shashidhara Veerabhadraiah
Sent: Monday, December 10, 2018 3:19 PM
To: Philip Race HYPERLINK
"mailto:philip.r...@oracle.com;
Cc: Prasanta Sadhukhan HYPERLINK
"mailto:prasanta.sadhuk...@oracle.com;;
HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: RE: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Hi Phil, Please find the updated Webrev.
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.03/
Thanks and regards,
Shashi
From: Philip Race
Sent: Friday, December 7, 2018 8:54 PM
To: Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com>
Cc: Prasanta Sadhukhan mailto:prasanta.sadhuk...@oracle.com"prasanta.sadhuk...@oracle.com>; HYPERLINK
"mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
First off, I think the high order bit is that if a non-null array reference is
returned there are NO
null String entries in it. Whether a zero length or null array is returned when
there are no printers
is the less important issue.
However an empty array also tells you there are no printers, and you are less
likely to get an NPE from that.
It is arguably easier for the caller, if they don't need the extra null check
first.
FWIW the javax.print public APIs return zero length arrays instead of null and
applications seem to survive :-)
I don't know what you mean by :
> (And anyway we need to handle the first null string reference)?
If you are referring to a small matter of coding in the native layer, then that
is not an insurmountable problem.
Basically if there are problems getting names or whatever in the native layer,
handle it THERE, don't make
everyone else have to deal with it.
One caveat: JNI calls can theoretically fail due to OOME .. in such a case we
are doomed and
the main goal is to not crash in native and to obey all JNI rules. What is
returned in that case
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.
-phil
On 12/6/18, 8:10 PM, Shashidhara Veerabhadraiah wrote:
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?
Thanks and regards,
Shashi
From: Phil Race
Sent: Thursday, December 6, 2018 2:35 AM
To: Shashidhara Veerabhadraiah HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com;;
Prasanta Sadhukhan HYPERLINK
"mailto:prasanta.sadhuk...@oracle.com;;
HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
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
st
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Sorry .. this got lost ..
I don't understand this line :
253 jobjectArray emptyArray = env->NewObjectArray(1, clazz, NULL);
This is returning an array of length 1 and element 0 is NULL.
I think you want
env->NewObjectArray(0, clazz, NULL); and I don't see why you need to
create it there instead you can just have 304 if (nameArray != NULL) {
305 return nameArray;
306 } else {
307 return env->NewObjectArray(0, clazz, NULL);
308 } 449 if (prevRemotePrinters != null) { shouldn't this be 449 if
(prevRemotePrinters != null && prevRemotePrinters.length > 0) { ? -phil.
On 12/10/18 10:19 PM, Shashidhara Veerabhadraiah wrote:
Hi Phil, I have updated with small code updates:
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.04/
Thanks and regards,
Shashi
*From:*Shashidhara Veerabhadraiah
*Sent:* Monday, December 10, 2018 3:19 PM
*To:* Philip Race
*Cc:* Prasanta Sadhukhan ;
2d-dev@openjdk.java.net
*Subject:* RE: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print
tests after JDK-8153732
Hi Phil, Please find the updated Webrev.
http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.03/
Thanks and regards,
Shashi
*From:*Philip Race
*Sent:* Friday, December 7, 2018 8:54 PM
*To:* Shashidhara Veerabhadraiah
<mailto:shashidhara.veerabhadra...@oracle.com>>
*Cc:* Prasanta Sadhukhan <mailto:prasanta.sadhuk...@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
First off, I think the high order bit is that if a non-null array
reference is returned there are NO
null String entries in it. Whether a zero length or null array is
returned when there are no printers
is the less important issue.
However an empty array also tells you there are no printers, and you
are less likely to get an NPE from that.
It is arguably easier for the caller, if they don't need the extra
null check first.
FWIW the javax.print public APIs return zero length arrays instead of
null and applications seem to survive :-)
I don't know what you mean by :
> (And anyway we need to handle the first null string reference)?
If you are referring to a small matter of coding in the native layer,
then that is not an insurmountable problem.
Basically if there are problems getting names or whatever in the
native layer, handle it THERE, don't make
everyone else have to deal with it.
One caveat: JNI calls can theoretically fail due to OOME .. in such a
case we are doomed and
the main goal is to not crash in native and to obey all JNI rules.
What is returned in that case
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.
-phil
On 12/6/18, 8:10 PM, Shashidhara Veerabhadraiah wrote:
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?
Thanks and regards,
Shashi
*From:*Phil Race
*Sent:* Thursday, December 6, 2018 2:35 AM
*To:* Shashidhara Veerabhadraiah
<mailto:shashidhara.veerabhadra...@oracle.com>; Prasanta Sadhukhan
<mailto:prasanta.sadhuk...@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
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/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.02/>
The big problem was that I was not able
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi Phil, I have updated with small code updates: http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.04/ Thanks and regards, Shashi From: Shashidhara Veerabhadraiah Sent: Monday, December 10, 2018 3:19 PM To: Philip Race Cc: Prasanta Sadhukhan ; 2d-dev@openjdk.java.net Subject: RE: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 Hi Phil, Please find the updated Webrev. http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.03/ Thanks and regards, Shashi From: Philip Race Sent: Friday, December 7, 2018 8:54 PM To: Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com> Cc: Prasanta Sadhukhan mailto:prasanta.sadhuk...@oracle.com"prasanta.sadhuk...@oracle.com>; HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 First off, I think the high order bit is that if a non-null array reference is returned there are NO null String entries in it. Whether a zero length or null array is returned when there are no printers is the less important issue. However an empty array also tells you there are no printers, and you are less likely to get an NPE from that. It is arguably easier for the caller, if they don't need the extra null check first. FWIW the javax.print public APIs return zero length arrays instead of null and applications seem to survive :-) I don't know what you mean by : > (And anyway we need to handle the first null string reference)? If you are referring to a small matter of coding in the native layer, then that is not an insurmountable problem. Basically if there are problems getting names or whatever in the native layer, handle it THERE, don't make everyone else have to deal with it. One caveat: JNI calls can theoretically fail due to OOME .. in such a case we are doomed and the main goal is to not crash in native and to obey all JNI rules. What is returned in that case 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. -phil On 12/6/18, 8:10 PM, Shashidhara Veerabhadraiah wrote: 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? Thanks and regards, Shashi From: Phil Race Sent: Thursday, December 6, 2018 2:35 AM To: Shashidhara Veerabhadraiah HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com;; Prasanta Sadhukhan HYPERLINK "mailto:prasanta.sadhuk...@oracle.com;; HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 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: HYPERLINK "http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.02/"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 HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com;; Prasanta Sadhukhan HYPERLINK "mailto:prasanta.sadhuk...@oracle.com;; HYPERLINK "mailto:2d-dev@openjdk.java.net"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*
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi Phil, Please find the updated Webrev. http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.03/ Thanks and regards, Shashi From: Philip Race Sent: Friday, December 7, 2018 8:54 PM To: Shashidhara Veerabhadraiah Cc: Prasanta Sadhukhan ; 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 First off, I think the high order bit is that if a non-null array reference is returned there are NO null String entries in it. Whether a zero length or null array is returned when there are no printers is the less important issue. However an empty array also tells you there are no printers, and you are less likely to get an NPE from that. It is arguably easier for the caller, if they don't need the extra null check first. FWIW the javax.print public APIs return zero length arrays instead of null and applications seem to survive :-) I don't know what you mean by : > (And anyway we need to handle the first null string reference)? If you are referring to a small matter of coding in the native layer, then that is not an insurmountable problem. Basically if there are problems getting names or whatever in the native layer, handle it THERE, don't make everyone else have to deal with it. One caveat: JNI calls can theoretically fail due to OOME .. in such a case we are doomed and the main goal is to not crash in native and to obey all JNI rules. What is returned in that case 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. -phil On 12/6/18, 8:10 PM, Shashidhara Veerabhadraiah wrote: 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? Thanks and regards, Shashi From: Phil Race Sent: Thursday, December 6, 2018 2:35 AM To: Shashidhara Veerabhadraiah HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com;; Prasanta Sadhukhan HYPERLINK "mailto:prasanta.sadhuk...@oracle.com;; HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 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: HYPERLINK "http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.02/"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 HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com;; Prasanta Sadhukhan HYPERLINK "mailto:prasanta.sadhuk...@oracle.com;; HYPERLINK "mailto:2d-dev@openjdk.java.net"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
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
First off, I think the high order bit is that if a non-null array
reference is returned there are NO
null String entries in it. Whether a zero length or null array is
returned when there are no printers
is the less important issue.
However an empty array also tells you there are no printers, and you are
less likely to get an NPE from that.
It is arguably easier for the caller, if they don't need the extra null
check first.
FWIW the javax.print public APIs return zero length arrays instead of
null and applications seem to survive :-)
I don't know what you mean by :
> (And anyway we need to handle the first null string reference)?
If you are referring to a small matter of coding in the native layer,
then that is not an insurmountable problem.
Basically if there are problems getting names or whatever in the native
layer, handle it THERE, don't make
everyone else have to deal with it.
One caveat: JNI calls can theoretically fail due to OOME .. in such a
case we are doomed and
the main goal is to not crash in native and to obey all JNI rules. What
is returned in that case
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.
-phil
On 12/6/18, 8:10 PM, Shashidhara Veerabhadraiah wrote:
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?
Thanks and regards,
Shashi
*From:*Phil Race
*Sent:* Thursday, December 6, 2018 2:35 AM
*To:* Shashidhara Veerabhadraiah
; Prasanta Sadhukhan
; 2d-dev@openjdk.java.net
*Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print
tests after JDK-8153732
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/
<http://cr.openjdk.java.net/%7Esveerabhadra/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
<mailto:shashidhara.veerabhadra...@oracle.com>; Prasanta Sadhukhan
<mailto:prasanta.sadhuk...@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
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/
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.01/>
Thanks and regards,
Shashi
*From:*Phil Race
*Sent:* T
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
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? Thanks and regards, Shashi From: Phil Race Sent: Thursday, December 6, 2018 2:35 AM To: Shashidhara Veerabhadraiah ; Prasanta Sadhukhan ; 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 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 HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com;; Prasanta Sadhukhan HYPERLINK "mailto:prasanta.sadhuk...@oracle.com;; HYPERLINK "mailto:2d-dev@openjdk.java.net"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 HYPERLINK "mailto:prasanta.sadhuk...@oracle.com;; Shashidhara Veerabhadraiah HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com;; HYPERLINK "mailto:2d-dev@openjdk.java.net"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
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
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
; Prasanta Sadhukhan
; 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
<mailto:prasanta.sadhuk...@oracle.com>; Shashidhara Veerabhadraiah
<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
<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:
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi Sergey, It is true that if there are no printers connected to the system we
should be getting the exception but all of windows system at least contains
'Fax', 'Microsoft print to PDF' as default printers in the printers list.
Ideally it is true that a null return case must have been handled.
So I simulated this effect by setting cReturned to zero causing a NPE to
trigger with the old code and then applied the fix and found that there is no
NPE anymore.
Thanks and regards,
Shashi
-Original Message-
From: Sergey Bylokhov
Sent: Tuesday, December 4, 2018 11:42 AM
To: Shashidhara Veerabhadraiah ; Philip
Race ; Prasanta Sadhukhan
; 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after
JDK-8153732
Hi, Shashi.
> 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.
If you cannot reproduce this exception on the system w/o printers, then it is
interesting what will return this code:
259 ::EnumPrinters(PRINTER_ENUM_LOCAL | PRINTER_ENUM_CONNECTIONS,
260NULL, 4, pPrinterEnum, cbNeeded, ,
261);
262
263 if (cReturned > 0) {}
what will happen if you try to print something to the printer which is returned
in the case above?
--
Best regards, Sergey.
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi, Shashi.
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.
If you cannot reproduce this exception on the system w/o
printers, then it is interesting what will return this code:
259 ::EnumPrinters(PRINTER_ENUM_LOCAL | PRINTER_ENUM_CONNECTIONS,
260NULL, 4, pPrinterEnum, cbNeeded, ,
261);
262
263 if (cReturned > 0) {}
what will happen if you try to print something to the printer
which is returned in the case above?
--
Best regards, Sergey.
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
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 ; Prasanta Sadhukhan ; 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 HYPERLINK "mailto:prasanta.sadhuk...@oracle.com;; Shashidhara Veerabhadraiah HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com;; HYPERLINK "mailto:2d-dev@openjdk.java.net"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 HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com;; HYPERLINK "mailto:swing-...@openjdk.java.net"swing-...@openjdk.java.net; HYPERLINK "mailto:2d-dev@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, HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com"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 poin
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 ; Shashidhara
Veerabhadraiah ;
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
<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 currentRemo
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
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 ; Shashidhara
Veerabhadraiah ; 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 HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com;;
HYPERLINK "mailto:swing-...@openjdk.java.net"swing-...@openjdk.java.net;
HYPERLINK "mailto:2d-dev@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, HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com"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: HYPERLINK
"http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.00/"http://cr.openjdk.java.net/~sveerabhadra/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
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
; 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
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
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
; 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
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
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 ;
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, HYPERLINK
"mailto:shashidhara.veerabhadra...@oracle.com"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: HYPERLINK
"http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.00/"http://cr.openjdk.java.net/~sveerabhadra/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
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 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/
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
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.
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/
