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

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to