Hi Sean,
Getting back to this review...
On 9/26/2019 1:55 PM, Sean Mullan wrote:
On 9/26/19 4:20 PM, Sean Mullan wrote:
Would you prefer I revise the patch where there are multiple
SuppressWarnings("serial") on fields to put a single one on the
class instead?
Yes, but only in the cases where we are clearly using some form of
alternate serialization like ASN.1 encoding. I need to double-check
the review again (it's a bit more time consuming because I have to
look at the code in more detail), but the two that I spotted so far are:
src/java.base/share/classes/sun/security/x509/X509CertImpl.java
src/java.security.jgss/share/classes/sun/security/krb5/internal/KRBError.java
(from the JDK-8231368 review)
Ok, I double-checked everything. The only other class in the webrev
that uses an alternate serial form is:
sun/security/x509/X509Key.java
but since that only has one field that is not Serializable, it
probably is ok to leave as-is.
I've put the updated webrev at:
http://cr.openjdk.java.net/~darcy/8231262.1/
The difference from the first webrev is I marked a field in X509Key.java
as transient; diff of webrev patches:
> --- old/src/java.base/share/classes/sun/security/x509/X509Key.java
2019-10-08 18:37:02.143999531 -0700
> +++ new/src/java.base/share/classes/sun/security/x509/X509Key.java
2019-10-08 18:37:01.815999531 -0700
> @@ -84,7 +84,7 @@
214,215c214,215
< + @SuppressWarnings("serial") // Not statically typed as Serializable
< private BitArray bitStringKey = null;
---
> - private BitArray bitStringKey = null;
> + private transient BitArray bitStringKey = null;
For src/java.base/share/classes/sun/security/x509/X509CertImpl.java, I
don't see any of the read* or write* methods associated with
serialization defined in the class. I would seem clearer to me to having
the @SuppressWarning("serial") at each field, but if you'd prefer to
have it at the class level, I'll defer to your judgement on the matter.
FYI, of the files being modified only
src/java.base/share/classes/javax/security/auth/Subject.java uses
readFields or putFields.
Thanks,
-Joe
