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

On Sun, 2011-11-20 at 20:57 -0500, Adam Young wrote: > 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 ACK