1:31 p.m.
This patch brings down the warnings from 3500 to 2981.
Ticket #2
http://fedorapeople.org/gitweb?p=edewata/public_git/pki.git;a=commitdiff;...
Passed smoke test.
--
Endi S. Dewata
7:26 p.m.
Went through the whole thing and appears to be mostly straightforward application of
generics
in order to remove many warnings. Just a couple of things I noticed:
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.
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.
Also, one more thing.
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.
thanks,
jack
----- Original Message -----
From: "Endi Sukma Dewata" <edewata(a)redhat.com>
To: pki-devel(a)redhat.com
Sent: Tuesday, February 7, 2012 11:31:37 AM
Subject: [Pki-devel] [PATCH] 15 Added generics (part 4).
This patch brings down the warnings from 3500 to 2981.
Ticket #2
http://fedorapeople.org/gitweb?p=edewata/public_git/pki.git;a=commitdiff;...
Passed smoke test.
--
Endi S. Dewata
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
12:17 p.m.
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
12:33 p.m.
On 2/13/2012 12:17 PM, Endi Sukma Dewata wrote:
I've updated a the patch.
It's available here:
http://fedorapeople.org/gitweb?p=edewata/public_git/pki.git;a=commitdiff;...
--
Endi S. Dewata
2:05 p.m.
On 2/13/2012 12:33 PM, Endi Sukma Dewata wrote:
On 2/13/2012 12:17 PM, Endi Sukma Dewata wrote:
> I've updated a the patch.
It's available here:
http://fedorapeople.org/gitweb?p=edewata/public_git/pki.git;a=commitdiff;...
Rebased.
http://fedorapeople.org/gitweb?p=edewata/public_git/pki.git;a=commitdiff;...
--
Endi S. Dewata
4:21 p.m.
ACK
I applied the patch and did some simple tests as well as revoking a few certificates
and updating and viewing the CRL's. Everything seems ok.
Noted that the invoking of the generic methods has been changed to not require the cast
that was previously there.
One minor commented out line I found. Fix if you like:
line 2311 of the patch.
----- Original Message -----
From: "Endi Sukma Dewata" <edewata(a)redhat.com>
To: "John Magne" <jmagne(a)redhat.com>
Cc: pki-devel(a)redhat.com
Sent: Tuesday, February 14, 2012 12:05:09 PM
Subject: Re: [Pki-devel] [PATCH] 15 Added generics (part 4).
On 2/13/2012 12:33 PM, Endi Sukma Dewata wrote:
On 2/13/2012 12:17 PM, Endi Sukma Dewata wrote:
> I've updated a the patch.
It's available here:
http://fedorapeople.org/gitweb?p=edewata/public_git/pki.git;a=commitdiff;...
Rebased.
http://fedorapeople.org/gitweb?p=edewata/public_git/pki.git;a=commitdiff;...
--
Endi S. Dewata
4:35 p.m.
On 2/14/2012 4:21 PM, John Magne wrote:
ACK
Thanks. Pushed to master.
I applied the patch and did some simple tests as well as revoking a
few certificates
and updating and viewing the CRL's. Everything seems ok.
Noted that the invoking of the generic methods has been changed to not require the cast
that was previously there.
One minor commented out line I found. Fix if you like:
line 2311 of the patch.
Yes, that's the code that I mentioned before that I was going to keep
for documentation.
--
Endi S. Dewata
4862
days inactive
4869
days old
6 comments
2 participants
participants (2)
-
Endi Sukma Dewata -
John Magne