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(a)redhat.com
>>
https://www.redhat.com/mailman/listinfo/pki-devel
>
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel