[Pki-devel] [PATCH] 0012-Dead-code-removal

Adam Young ayoung at redhat.com
Wed Nov 16 15:33:23 UTC 2011


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 at redhat.com
>> https://www.redhat.com/mailman/listinfo/pki-devel
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: dogtag-admiyo-0012-1-Dead-code-removal.patch
Type: text/x-patch
Size: 47201 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20111116/736024ab/attachment.bin>


More information about the Pki-devel mailing list