Artem,
please see updated review:
http://cr.openjdk.java.net/~snikandrova/8054536/webrev.02/
<http://cr.openjdk.java.net/%7Esnikandrova/8054536/webrev.02/>
Thanks,
Svetlana
On 15.07.2016 21:32, Artem Smotrakov wrote:
Hi Svetlana,
The webrev looks fine to me. Please see one more comment inline.
On 07/15/2016 11:12 AM, Svetlana Nikandrova wrote:
Hi Artem,
ok, lets get rid of possible NPE in other methods too:
http://cr.openjdk.java.net/~snikandrova/8054536/webrev.01/
<http://cr.openjdk.java.net/%7Esnikandrova/8054536/webrev.01/>
I also noticed that encode(DerOutputStream) checks extensionId and
extensionValue for null, but encode(OutputStream) doesn't.
encode(OutputStream) might just call encode(DerOutputStream), so that
it always checks those fields for null. It also would remove some
duplicate code.
As for unnecessary try-catch in test I'd prefer to have it to
emphasis that we are checking for NPE.
Okay.
I'm not sure about "iff" but let it be, it seems like a right place
to use it.
Sure.
Artem
Thank you,
Svetlana
On 14.07.2016 19:35, Artem Smotrakov wrote:
Hi Svetlana,
I'll leave the main review to an official reviewer, but I have a
couple of comments.
There are a couple of other places in Extension.java where NPE may
occur:
- line 255: I see that "extensionId" is checked for null in other
methods, but not in getId()
- line 200: I see that "extensionValue" is checked for null in other
methids, but not in getValue()
Minor: try-catch is not really necessary.
I am not sure, but "iff" might mean "if and only if", so it may be
not a typo.
You may want to add @Override to a couple of methods since you
update Extension.java
Artem
On 07/14/2016 08:19 AM, Svetlana Nikandrova wrote:
Hello,
please review the fix for:
https://bugs.openjdk.java.net/browse/JDK-8054536
Webrev:
http://cr.openjdk.java.net/~snikandrova/8054536/webrev.00/
<http://cr.openjdk.java.net/%7Esnikandrova/8054536/webrev.00/>
Description:
Equals and hasCode methods of Extension use extensionId without
prior check for "null" (+ 1 mistype in equals javadoc).
Thank you,
Svetlana
