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
--
Jan Cholasta
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
