On 29.01.2016 14:49, Stanislav Laznicka wrote:
Hello,
I made some patches based on the Coverity report from 18.1.2016.
Cheers,
Standa
NACK
*) please describe issue in commit message instead of #coverity number
in all patches
PATCH: Removing dead code
LGTM
PATCH: Wrong assert
I'm not sure with this, please ask pspacek
PATCH: Searching for an attribute in a wrong object
Please file a ticket for this, if this is right, we might want to
backport patch
PATCH: Class attribute 'type' should be used instead
LGTM
PATCH: Removed identical branch
NACK, bytes and string are not the same type, this may cause issues in
python3
PATCH: completed_external might not have beed defined
LGTM
PATCH: Accessing attribute of a possible None value
LGTM
PATCH: Possible close/write into None or closed file
it looks like assertion overkill to me, would be better to rewrite the
logic IMO
PATCH: Fixed possible dereferencing of undefined objects
NACK
1)
- root_logger.error('Failed to get create new request.')
+ else:
+ raise RuntimeError('Failed to add a request to DBUS.'
raise original error, do not reraise new exception, it will lost traceback
2)
I would put return to else: block, and raise RuntimeError in try block,
then just add except (TypeError, RuntimeError)
Try to avoid broad exceptions.
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
