[Freeipa-devel] [PATCH 0386] private_ccache: Harden the removal of KRB5CCNAME env variable

[Tomas Babej]

On 11/23/2015 01:11 PM, Jan Cholasta wrote:
> On 23.11.2015 12:53, Tomas Babej wrote:
>> Hi,
>>
>> If the code within the private_ccache contextmanager does not
>> set/removes the KRB5CCNAME, the pop method will raise KeyError, which
>> will cause unnecessary termination of the code flow.
>>
>> Make sure the KRB5CCNAME is popped out of os.environ only if present.
>>
>> Tomas
> 
> NACK, use os.environ.pop('KRB5CCNAME', None) to safely remove it.

Yeah, well, both ways are equivalent, I found the if statement more
explicit. We can go with the suggested version, if it's more pleasing
though - patch is attached.

> 
> Also I don't think the comment is necessary, it's quite obvious what the
> code does.
> 

I don't think an explanatory comment can hurt, ever. Worst thing that
happens is that somebody is assured that he understands the code correctly.

Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0386-2-private_ccache-Harden-the-removal-of-KRB5CCNAME-env-.patch
Type: text/x-patch
Size: 1176 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151123/5eb2add8/attachment.bin>