[Pki-devel] Request for review: ECC support for pkisilent (CA, subCA, DRM, TKS, TPS, OCSP)

[Ade Lee]

Christina, 

The changes you have put in ComCrypto.java look great.  I did notice,
however, that you do a similar token assignment around line 710, but
still have the unnecessary cast, no check for null and wonky exception
handling.  Please fix this instance too.

Once that is done, we can ACK the patch.

Ade

On Tue, 2013-02-19 at 18:04 -0800, Christina Fu wrote:
> Ade,
> 
> Thank you for the review.
> 
> Per our discussion.  The agreed changes can now be found at:
> https://bugzilla.redhat.com/attachment.cgi?id=699762&action=diff&context=patch&collapsed=&headers=1&format=raw
> 
> In addition, I have added changes to the admin cert profile to accept ECC:
> https://bugzilla.redhat.com/attachment.cgi?id=699763&action=diff&context=patch&collapsed=&headers=1&format=raw
> 
> Also, I have now put the ECC setup instruction at this location:
> http://pki.fedoraproject.org/wiki/ECC_Setup_Instructions
> So the silentEC_readme.txt now only contains link to it:
> https://bugzilla.redhat.com/attachment.cgi?id=699764
> 
> thanks!
> Christina
> 
> On 02/19/2013 12:04 PM, Ade Lee wrote:
> > Comments:
> >
> > In general, the code changes look fine.  Just a couple of nitpicks ..
> >
> > 1. The variable save_p12 should be a boolean rather than a string.
> >
> > 2. In ComCrypto.java, you define setTokenName().  What about
> > getTokenName()?
> >
> > 3. In ComCrypto.java,on line 448, you cast token to (PK11Token) and on
> > 450, there is no cast.  Why the discrepancy?  Is the cast needed?
> >
> > 4. The exception handling in ComCrypto.java for lines 448/450 is pretty
> > wonky.  It seems like token could return null.  Could we improve the
> > exception handling in this case?
> >
> > Ade
> >
> > On Tue, 2The 013-02-19 at 11:26 -0800, Christina Fu wrote:
> >> This is a request for code review for the following feature bug:
> >> Bug 810967 - [RFE] ECC support for pkisilent
> >>
> >> the code changes can be found here:
> >> https://bugzilla.redhat.com/attachment.cgi?id=699587&action=diff&context=patch&collapsed=&headers=1&format=raw
> >>
> >> The 5 new templates and one readme instruction file can be found the
> >> the bug attachment.
> >>
> >> thanks!
> >> Christina
> >>
> >>
> >> _______________________________________________
> >> Pki-devel mailing list
> >> Pki-devel at redhat.com
> >> https://www.redhat.com/mailman/listinfo/pki-devel
>