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

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