Reworded the commits so that they better reflect what's going on in those.

On 01/29/2016 02:49 PM, Stanislav Laznicka wrote:
Hello,

I made some patches based on the Coverity report from 18.1.2016.

Cheers,
Standa



From 56bfba733321388190cf6df0ec0dfab5fff15996 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 29 Jan 2016 08:57:06 +0100
Subject: [PATCH 1/9] Removing dead code

---
 ipaclient/ipadiscovery.py | 3 ---
 ipalib/plugins/dns.py     | 3 ---
 ipatests/i18n.py          | 4 ++--
 3 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/ipaclient/ipadiscovery.py b/ipaclient/ipadiscovery.py
index 45a71e190e56d33d51d37f16ae61a7b4c28df521..39f3fcc98d06d7041a82ca7f5e6c240fe9a23983 100644
--- a/ipaclient/ipadiscovery.py
+++ b/ipaclient/ipadiscovery.py
@@ -401,9 +401,6 @@ class IPADiscovery(object):
                 else:
                     return [0, thost, lrealms[0]]
 
-            #we shouldn't get here
-            return [UNKNOWN_ERROR]
-
         except errors.DatabaseTimeout:
             root_logger.debug("LDAP Error: timeout")
             return [NO_LDAP_SERVER]
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 0a9fa181347514f0cc11c3ab4a3e19864df4eec0..1892d29ec9f87d3b3f955ff8df6b154961fbce36 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -800,9 +800,6 @@ class DNSRecord(Str):
         if value is None:
             return
 
-        if value is None:
-            return
-
         if not self.supported:
             return _('DNS RR type "%s" is not supported by bind-dyndb-ldap plugin') \
                      % self.rrtype
diff --git a/ipatests/i18n.py b/ipatests/i18n.py
index a83c5806ef8ceaf011ec54db18d0222769bad1b1..368712599063c3281b95bf46e48dea924fc7999e 100755
--- a/ipatests/i18n.py
+++ b/ipatests/i18n.py
@@ -755,7 +755,7 @@ def main():
         print('ERROR: no mode specified', file=sys.stderr)
         return 1
 
-    if options.mode == 'validate_pot' or options.mode == 'validate_po':
+    if options.mode in ('validate_pot', 'validate_po'):
         if options.mode == 'validate_pot':
             files = args
             if not files:
@@ -795,7 +795,7 @@ def main():
         else:
             return 0
 
-    elif options.mode == 'create_test' or 'test_gettext':
+    elif options.mode in ('create_test', 'test_gettext'):
         po_file = '%s.po' % options.test_lang
         pot_file = options.pot_file
 
-- 
2.5.0

From 131fdd331d2283a7fc406e7c03e9f8e67f99794d Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 29 Jan 2016 09:01:43 +0100
Subject: [PATCH 2/9] Wrong assert in ldapkeydb

Asserting 'ipk11id' attribute in different object than from which it
is later accessed
---
 ipapython/dnssec/ldapkeydb.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipapython/dnssec/ldapkeydb.py b/ipapython/dnssec/ldapkeydb.py
index 55c09c040a1790569924f506f520b8327775ee7c..aa04139348a42910ed20db5189db70e262c1d5c1 100644
--- a/ipapython/dnssec/ldapkeydb.py
+++ b/ipapython/dnssec/ldapkeydb.py
@@ -288,7 +288,7 @@ class LdapKeyDB(AbstractHSM):
             for attr in default_attrs:
                 key.setdefault(attr, default_attrs[attr])
 
-            assert 'ipk11id' in o, 'key is missing ipk11Id in %s' % key.entry.dn
+            assert 'ipk11id' in key, 'key is missing ipk11Id in %s' % key.entry.dn
             key_id = key['ipk11id']
             assert key_id not in keys, 'duplicate ipk11Id=0x%s in "%s" and "%s"' % (hexlify(key_id), key.entry.dn, keys[key_id].entry.dn)
             assert 'ipk11label' in key, 'key "%s" is missing ipk11Label' % key.entry.dn
-- 
2.5.0

From 528af83bee872b5722cdfef7c47e011f037db274 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 29 Jan 2016 09:03:11 +0100
Subject: [PATCH 3/9] Searching for an attribute in a wrong object

Checking for an attribute in a different object than it's
accessed later
---
 ipalib/plugins/permission.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 2312008fa6db646dd4e3e9598daf7ea48c867337..9c64fdf5f6a867a4eab1ccfcdba663d5e1214f08 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -473,7 +473,7 @@ class permission(baseldap.LDAPObject):
 
             if old_client:
                 for old_name, new_name in _DEPRECATED_OPTION_ALIASES.items():
-                    if new_name in entry:
+                    if new_name in rights:
                         rights[old_name] = rights[new_name]
                         del rights[new_name]
 
-- 
2.5.0

From 61826966e57d2b070ce3de25a9f6fc9cf2588ab5 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 29 Jan 2016 09:08:19 +0100
Subject: [PATCH 4/9] Builtin 'type' used wrong in Param class

The builting 'type' is used instead of the same-named class member
---
 ipalib/parameters.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index e46068c96564d9fad61cbc75b8fe4f8f40cca98b..8d174ebec28bc0e3b2762c5d8dc1d99c041c54c9 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -482,7 +482,7 @@ class Param(ReadOnly):
                     elif type(value) is str:
                         value = frozenset([value])
                 if (
-                    type(kind) is type and not isinstance(value, kind)
+                    type(kind) is self.type and not isinstance(value, kind)
                     or
                     type(kind) is tuple and not isinstance(value, kind)
                 ):
-- 
2.5.0

From 07e6dbadf84f7ea74ac7d6da5e1d3575d6b3b6b2 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 29 Jan 2016 09:11:03 +0100
Subject: [PATCH 5/9] Removed identical branch

Two branches are performing the same action.
---
 ipapython/ipautil.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 7949bdf05cd72c512e6c2bc24b1bc52012e63317..3d4e62556a13f20a32602b7e3156722bee9e61fa 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -258,10 +258,7 @@ def write_tmp_file(txt):
     return fd
 
 def shell_quote(string):
-    if isinstance(string, str):
-        return "'" + string.replace("'", "'\\''") + "'"
-    else:
-        return b"'" + string.replace(b"'", b"'\\''") + b"'"
+    return "'" + string.replace("'", "'\\''") + "'"
 
 
 if six.PY3:
-- 
2.5.0

From 5bf3365688284a4b90ea81b67c290523bb82f653 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 29 Jan 2016 09:16:04 +0100
Subject: [PATCH 6/9] completed_external might not have beed defined

Should the first condition fail, completed_external will be undefined in
the return.
---
 ipalib/plugins/baseldap.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 03a255ca1303091f8a9f08f6ae1d0f76281937a0..ffc0008a65d3ea52a084870591b867648c8dea36 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -451,12 +451,12 @@ def remove_external_post_callback(ldap, dn, entry_attrs, failed, completed,
 
     # Run through the failures and gracefully remove any member defined
     # as an external member.
+    completed_external = 0
     if memberattr in failed and membertype in failed[memberattr]:
         entry_attrs_ = ldap.get_entry(dn, [externalattr])
         dn = entry_attrs_.dn
         external_entries = entry_attrs_.get(externalattr, [])
         failed_entries = []
-        completed_external = 0
 
         for entry in failed[memberattr][membertype]:
             membername = entry[0].lower()
-- 
2.5.0

From d66b49a249dc655e7d7b0996b30530700ce4f5ea Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 29 Jan 2016 09:18:31 +0100
Subject: [PATCH 7/9] Accessing attribute of a possible None value

If no exceptions occur, e would be None
---
 ipatests/test_xmlrpc/xmlrpc_test.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/xmlrpc_test.py b/ipatests/test_xmlrpc/xmlrpc_test.py
index 8140364a53c20513fa6265997b8cd48e6140e62b..c3283f23b78fd8c0b77a7b4bc16278017f90eac6 100644
--- a/ipatests/test_xmlrpc/xmlrpc_test.py
+++ b/ipatests/test_xmlrpc/xmlrpc_test.py
@@ -355,9 +355,10 @@ class Declarative(XMLRPC_test):
         except Exception as e:
             pass
         if not expected(e, output):
+            cls_name = e.__class__.__name__ if e else None
             raise AssertionError(
                 UNEXPECTED % (cmd, name, args, options,
-                              e.__class__.__name__, e)
+                              cls_name, e)
             )
 
     def check_output(self, nice, cmd, args, options, expected, extra_check):
-- 
2.5.0

From 869a84d02455b24bf5c87c91d1940451250bffa2 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 29 Jan 2016 09:38:17 +0100
Subject: [PATCH 8/9] Possible close/write into None or closed file

The test awaits a perfect input resulting into several
possible None dereferences/writes to closed files.
---
 ipatests/test_xmlrpc/test_automount_plugin.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_automount_plugin.py b/ipatests/test_xmlrpc/test_automount_plugin.py
index 974a0f1f7ff227cfb97b877f1e889b454b47dc38..346baaf74e95725dd6373b56a4aeedca07094b3a 100644
--- a/ipatests/test_xmlrpc/test_automount_plugin.py
+++ b/ipatests/test_xmlrpc/test_automount_plugin.py
@@ -80,13 +80,19 @@ class AutomountTest(XMLRPC_test):
         for line in self.tofiles_output.splitlines():
             line = line.replace('/etc/',  '%s/' % conf_directory)
             if line.startswith(conf_directory) and line.endswith(':'):
+                assert(not current_file or current_file.closed), ('Malformed input file: '
+                       'opening file while previous is still open.')
                 current_file = open(line.rstrip(':'), 'w')
             elif '--------' in line:
+                assert current_file, 'Malformed input file: closing undefined file.'
                 current_file.close()
             elif line.startswith('maps not connected to '):
                 break
             else:
+                assert current_file, 'Malformed input file: writing into undefined file.'
+                assert not current_file.closed, 'Malformed input file: writing into closed file.'
                 current_file.write(line + '\n')
+        assert current_file, 'The input file does not contain any records of files to be opened.'
         current_file.close()
 
         self.failsafe_add(api.Object.automountlocation, self.locname)
-- 
2.5.0

From 7b6dd30dc3dd538657105cd8c6c07d712eb1b213 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 29 Jan 2016 10:25:24 +0100
Subject: [PATCH 9/9] Fixed possible dereferencing of undefined objects

Should bool(result[0]) be false, request would be undefined yet
dereferenced in function return
---
 ipapython/certmonger.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py
index f89ca0b7a1cbb9d34b0c044e30e213e7aa1c74fd..6318884c8ab05b2d5505bcda70db8329bd0ab9ce 100644
--- a/ipapython/certmonger.py
+++ b/ipapython/certmonger.py
@@ -318,8 +318,10 @@ def request_cert(nssdb, nickname, subject, principal, passwd_fname=None):
         if result[0]:
             request = _cm_dbus_object(cm.bus, cm, result[1], DBUS_CM_REQUEST_IF,
                                       DBUS_CM_IF, True)
-    except TypeError:
-        root_logger.error('Failed to get create new request.')
+        else:
+            raise RuntimeError('Failed to add a request to DBUS.')
+    except Exception:
+        root_logger.error('Failed to create a new request.')
         raise
     return request.obj_if.get_nickname()
 
@@ -356,6 +358,8 @@ def start_tracking(nickname, secdir, password_file=None, command=None):
         if result[0]:
             request = _cm_dbus_object(cm.bus, cm, result[1], DBUS_CM_REQUEST_IF,
                                       DBUS_CM_IF, True)
+        else:
+            raise RuntimeError('Failed to add a request to DBUS.')
     except TypeError as e:
         root_logger.error('Failed to add new request.')
         raise
-- 
2.5.0

-- 
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

Reply via email to