On 20/06/2013 22:59, Phil Race wrote:
On 6/20/2013 11:54 AM, Kurchi Hazra wrote:
Thanks Chris, webrev with the equals() changes then :
http://cr.openjdk.java.net/~khazra/8017109/webrev.01/
I am running this through jdk_awt and jdk_swing tests. If all is good,
I'll push through tl then.
Not sure which tests those are.
I think the only way this will get exercised is if you run it through
printing tests on Linux or Solaris 11 as this code is used only
by CUPS printing. Anything else you do is probably a waste of time.
I think automated tests will tell you very little as they are often
run on systems without printers and may not even hit this code,
unless they actually print.
I am not sure why equals is only testing the name, but it looks
like the name is used as a hashmap key with instances as values
so if there's more than one with differences in the other fields,
there are bigger problems.
Really I'd prefer you push this via 2d (not awt as Chris suggested)
but I won't insist if it will cause pain for you.
Sorry, my confusion, 2d would be better.
I don't think it really matters to Kurchi where this gets pushed, so
long as there is some advantage of getting it into 2d, rather than tl (
more testing before integration, PIT, etc ).
Alternatively, someone from the "2d" area could sponsor Kurchi's
changeset into 2d.
-Chris.
-phil.
Thanks,
- Kurchi
On 6/20/2013 1:37 AM, Chris Hegarty wrote:
On 06/19/2013 10:38 PM, Kurchi Hazra wrote:
Hi,
The class src/solaris/classes/sun/print/AttributeClass.java overrides
Object.equals() but not Object.hashCode() because of which
we get a warning everytime we build jdk8/tl. Here is a suggestion to
remove the warning:
http://cr.openjdk.java.net/~khazra/8017109/webrev.00/
<http://cr.openjdk.java.net/%7Ekhazra/8017109/webrev.00/>
Your changes to hashCode are in line with what the equals method is
doing. So, they look fine to me.
I was also wondering whether the implementation of the equals() method
is correct, and whether it is instead meant to be:
diff -r eb1a3c50a2a9 src/solaris/classes/sun/print/AttributeClass.java
--- a/src/solaris/classes/sun/print/AttributeClass.java Tue Jun 18
14:11:45 2013 -0700
+++ b/src/solaris/classes/sun/print/AttributeClass.java Wed Jun 19
14:20:27 2013 -0700
@@ -250,9 +250,8 @@
public boolean equals(Object obj) {
return
- obj != null &&
obj instanceof AttributeClass &&
- obj.toString().equals (((AttributeClass) obj).toString());
+ toString().equals (((AttributeClass) obj).toString());
}
D'oh!
public String toString() {
What is the preferable way of getting this change into the code - I can
push via tl. What are the tests I would need to run?
It probably should go through the awt forest, but since the scope is
limited I see no problem running it through TL.
-Chris.
If this discussion belongs elsewhere, let me know.
Thanks for your help,
- Kurchi
