[Freeipa-devel] [PATCH] 1067 clear out certmonger requests

[Rob Crittenden]

Jan Cholasta wrote:
> On 29.10.2012 20:11, Rob Crittenden wrote:
>> Jan Cholasta wrote:
>>> Hi,
>>>
>>> On 24.10.2012 21:22, Rob Crittenden wrote:
>>>> If uninstall fails in certain ways it is possible that some
>>>> certificates
>>>> could still be tracked by certmonger (even if the NSS database is now
>>>> gone). This will loop through the directories we care about and warn
>>>> the
>>>> user if there is anything left over.
>>>>
>>>> I added some basic test instructions to the ticket.
>>>>
>>>> rob
>>>>
>>>
>>> You should check the return value of find_request_value, it can be None
>>> in case of error.
>>>
>>> I would prefer if you used "os.path.join(REQUEST_DIR, file)" instead of
>>> "'%s/%s' % (REQUEST_DIR, file)".
>>>
>>> There is a trailing whitespace in the patch on line 75.
>>>
>>> Honza
>>>
>>
>> fixed.
>>
>> rob
>
> This will still break if find_request_value returns None:
>
> +    for file in fileList:
> +        rv = find_request_value(os.path.join(REQUEST_DIR, file),
> +                                'cert_storage_location')
> +        if rv:
> +            rv = os.path.abspath(rv)
> +        if rv.rstrip() == dir:
> +            id = find_request_value(os.path.join(REQUEST_DIR, file),
> 'id').rstrip()
> +            if id is not None:
> +                reqid.append(id)
>
> I would suggest to do it like this instead:
>
> +    for file in fileList:
> +        rv = find_request_value(os.path.join(REQUEST_DIR, file),
> +                                'cert_storage_location')
> +        if rv is None:
> +            continue
> +        rv = os.path.abspath(rv).rstrip()
> +        if rv != dir:
> +            continue
> +        id = find_request_value(os.path.join(REQUEST_DIR, file), 'id')
> +        if id is not None:
> +            reqid.append(id.rstrip())
>
> Honza
>

OK, great suggestion, sorry I didn't grok it sooner. Updated patch attached.

rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-1067-3-certmonger.patch
Type: text/x-diff
Size: 4433 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20121031/4e07df5c/attachment.bin>