Jan Cholasta wrote:
On 31.10.2012 16:28, Rob Crittenden wrote:
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

You missed one spot:

+        id = find_request_value(os.path.join(REQUEST_DIR, file), 'id')
+        if id is not None:
+            reqid.append(id.rstrip())

Also there is a trailing whitespace on line 73 of the patch.

Once you fix this, ACK.

Honza


Done, pushed to master and ipa-3-0

rob

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

Reply via email to