[Pki-devel] [PATCH] 15 Added generics (part 4).

[Endi Sukma Dewata]

John,

I've updated a the patch.

On 2/8/2012 7:26 PM, John Magne wrote:
> 1. Line 346 of patch. Commented out a line instead of removing. Was there perhaps a reason for this?
> //DerOutputStream outChain = new DerOutputStream();
> 2. Line 1467. Commented out line.           //String localAgents = req.getParameter("localAgents");
> 3. Line 2313. Commented out line.             //String tag = value.substring(0, i);
> 4. Line 2848. Commented out line.            //BigInt privateKeyVersion = privateKeyDerIn.getInteger();
>
> I found some instances of commented out lines as above. Those are the ones I was able to find.

These are unused variables. I've revised the patch and remove them 
except a few for documentation. For example the following code describes 
that the value consists of a tag and a val:

             //String tag = value.substring(0, i);
             String val = value.substring(i + 1);

The following code is actually needed in order to read the stream 
properly so I've restored it:

     BigInt privateKeyVersion = privateKeyDerIn.getInteger();

> Also, the following construct I was not sure about. Loading it into Eclipse, there are no flags.
> If you have a couple thoughts on this that would be great.
>
> -    protected KeySpec engineGetKeySpec(Key key, Class keySpec)
> +    @SuppressWarnings("unchecked")
> +    protected<T extends KeySpec>  T engineGetKeySpec(Key key, Class<T>  keySpec)
>               throws InvalidKeySpecException {
>
> In this instance it looks like the actual code calling this method has not changed. That could be totally normal.

I was fixing it to match the method signature in the parent class. With 
generic method we're supposed to provide the actual type for T (i.e. 
DSAPublicKeySpec) when calling the method:

     DSAPublicKeySpec dsaPubKeySpec =
         this.<DSAPublicKeySpec>engineGetKeySpec(...);

But the compiler can infer the T from the return type and the parameters 
so the code can be simplified further:

     DSAPublicKeySpec dsaPubKeySpec = engineGetKeySpec(...);

> I noticed that much of the fixes touch the CRL stuff. It would be cool to run a few quick tests consisting of
> revoking some certs and using the agent UI to inspect the CRL's to see if everything looks ok.

Yes, I've done that as part of the smoke test.

Thanks.

-- 
Endi S. Dewata
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dogtag-edewata-0015-1-Added-generics-part-4.patch
Type: text/x-patch
Size: 188930 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20120213/c0b02fbd/attachment.bin>