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