[Pki-devel] [PATCH] 0012-Dead-code-removal
Adam Young
ayoung at redhat.com
Mon Nov 21 01:57:12 UTC 2011
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>
More information about the Pki-devel
mailing list