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

[Adam Young]

On 11/16/2011 10:33 AM, Adam Young wrote:
> 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
>>
>
>
>
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
Is that an ACK?


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20111120/ae939f47/attachment.htm>