On Thu, 18 Nov 2021 19:27:30 GMT, Martin Balao <mba...@openjdk.org> wrote:

> 
> 
> Hi @valeriepeng ,
> 
> Some comments and questions regarding Webrev.01:
> 
>     * P11Key.java
> 
>     * Would you consider replacing the 'Internal' suffix with 'Opaque'? I 
> believe the term 'opaque' better reflects what these keys are: you cannot see 
> their values -thus, opaque- but you can use them as-is. 'Internal' gives me 
> the impression of something not supposed to be exposed; and this is not 
> exactly the case.
> 
>     * Why are P11RSAPrivateKeyInternal::n, P11RSAPrivateKey::<e, d, p, q, pe, 
> qe, coeff>, etc. now transient?
> 
>     * Now that P11RSAPrivateKey::n private field was removed, the extra 
> PKCS#11 lib call I mentioned in Webrev.00 is not possible anymore. The 
> override of ::getModulus looks good to me, though, for the reasons you said.
> 
>     * Can P11RSAPrivateNonCRTKey use the protected 'n' field from its parent 
> instead of declaring a private one?
> 
>     * P11Signature.java
> 
>     * 'long errorCode = e.getErrorCode();' -> Looks to me that this change 
> was included into the Webrev by mistake, and is part of JDK-8236512.
> 
> 
> Thanks, Martin.-

Either internal or opaque suffix works for me.  Fields aren't really being 
written out into the serialized bytes are "transient". IIRC, the serialized 
bytes are the "encoding" instead of the fields, we should mark these fields 
transient. As for the P11RSAPrivateNonCRTKey, yes, it should use the 'n' from 
parent instead of its own. I missed it when working on webrev.01. As for 'long 
errorCode = e.getErrorCode();' in P11Signature.java, yes, it should have been 
removed. I was manually merging the changes when refreshing the source tree and 
missed to remove this line.

Thanks,
Valerie

-------------

PR: https://git.openjdk.java.net/jdk/pull/4961

Reply via email to