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

Ade Lee alee at redhat.com
Mon Nov 21 14:49:01 UTC 2011


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

ACK




More information about the Pki-devel mailing list