[Freeipa-devel] [PATCH 0013-0021] Coverity patches

[Martin Basti]

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.




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160129/f8080731/attachment.htm>