On 03/02/2015 09:24 AM, Florian Weimer wrote:
On 02/20/2015 11:23 PM, Sean Mullan wrote:
On 02/17/2015 09:30 AM, Florian Weimer wrote:
On 02/16/2015 11:13 PM, Sean Mullan wrote:
Based on that, PolicyQualifierInfo should have implemented value-based
equals() and hashCode(), and the identity-based set is just a bug.
(But
the requirement I cited is a stronger requirement the Set would not
enforce.)
However, I think it's too late to fix this bug now. That's why I just
added the identity counter. If you want the behavioral change instead,
I can implement that as well.
Maybe it's not too late. This is not a commonly used class, and the
compatibility risk is probably fairly low. If you code up the changes, I
can file a CCC on your behalf.
Updated webrev: <http://cr.openjdk.java.net/~fweimer/8072394/webrev.01/>
You need to add a description for the overridden equals/hashCode
methods, ex:
Okay, I didn't know that.
I don't like specifying the hash code so strongly.
What about adding it as an @implNote?
But if we want to
fix it at Arrays.hashCode(byte[]), it's probably better to implement
Comparable as well, to aid the collision resolution in HashMap. The new
webrev does that:
<http://cr.openjdk.java.net/~fweimer/8072394/webrev.02/>
There is a typo on line 203: s/orded/ordered
Also, while you are in there, can you add an @Override to toString.
I can take care of filing an internal CCC and will let you know when
that is approved or if there are any questions.
--Sean
I also compute the hashcode unconditionally in the constructor because
PolicyQualifierInfo objects are used with sets, so their hashcode is so
often needed.
Funny how I wanted to make things go faster, and ended up introducing a
slowdown. But correctness over performance.
Most policy qualifier sets in X.509 certificates seem to have just one
element, so I looked at optimizing that, but there's a lot of copying
going on inside the implementation, and I'd have to special-case the one
element case in multiple locations. Switching from a HashSet to a
TreeSet would be externally visible, so that doesn't seem to be a good
idea, either.
We need an optimized immutable set class in the JDK. :-)
