On 11/15/2011 01:11 AM, Ade Lee wrote:

This is a review of this patch and the subsequent one removing
unnecessary blocks.

CMCAuth.java: Can you explain why this code removal is correct?

At this point in the code,  cert can only be null.  The only code path that assigns a value to cert has a return after it,  and cannot reach this point.  Thus,  the code executed when cert != null is unreachable

CAAdminServlet.java : code should be commented out, rather than removed.

Disagree.  If this code has never been run,  it is unnecessary.  Lets not put dead code into the source tree.

HashEnrollServlet.java : remove the outer conditional as well.

Done

DBSubsystem.java: some important comments are removed, they should not
be removed.

Done

FileAsString.java - does the proposed code removal introduce a resource
leak?

No.  FileReader  can throw a file not found exception.  But BufferedReader only throws an IllegalArgumentException,  which wouldn't be caught by that catch block anyway.

KeyRecoveryAuthority.java: please explain why the proposed code removal
is correct.  It certainly looks wrong.

I agree that the change looks wrong.  I put it back in,  and Eclipse did not tag it as dead code.

Ade

On Wed, 2011-11-09 at 19:50 -0500, Adam Young wrote:

_______________________________________________
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel

_______________________________________________
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel