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

[Jan Cholasta]

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

-- 
Jan Cholasta