On Tue, 30 Nov 2021 19:48:19 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> Hmm, thinking more about "internal"/"opaque", given this is naming for the >> parent, maybe "internal" is more correct. The non-sensitive keys are >> encapsulated by the children classes and is still an instance of the parent. >> If you name the parent class w/ "opaque" suffix, it looks >> misleading/confusing. The opaqueness is implied by the implementation >> instead of the properties of the objects. > >> Hi @valeriepeng , >> >> Yes, I just verified how serialization works for P11Keys and your >> 'transient' change makes sense to me now. >> >> I see your point about Internal/Opaque. Keep 'Internal' then if you prefer. >> The whole inheritance relationship between these classes sounds a bit weird >> to me, beyond the names we call them. I wouldn't suggest any additional >> changes there now, though. >> >> It looks to me that the only 2 changes expected for Webrev.02 are: 1) >> P11RSAPrivateNonCRTKey to use the parent's protected 'n'; and 2) removal of >> 'long errorCode = e.getErrorCode();'. >> >> Now that we almost have the changeset ready, I'm not sure of how to proceed >> with the push. Do you want me to commit Webrev.02 in my own branch and use >> the 'Co-authored-by:' header? If we do that, do we still need an additional >> Reviewer? Do you prefer to have your own branch? Please let me know of how >> to move forward. >> >> Martin.- > > You can just incorporate the changes on your branch and proceeds with me > being the reviewer. The webrev that I sent u is just an easier way to express > the comments/feedbacks. > > Thanks, > Valerie Hi @valeriepeng , Please take a look at the latest change based on your comments. No regressions found in 'jdk/sun/security/pkcs11'. Thanks, Martin.- ------------- PR: https://git.openjdk.java.net/jdk/pull/4961
