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

Adam Young ayoung at redhat.com
Mon Nov 21 18:38:33 UTC 2011


On 11/21/2011 09:49 AM, Ade Lee wrote:
> 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
>
Commited to trunk.  To to the PKISIlent restructuring,  I had to move 
the ArgParser code by hand,  but the change matches what was done in the 
patch.




More information about the Pki-devel mailing list