[PATCH] a couple of relatively interesting python fixes
Douglas Bagnall
douglas.bagnall at catalyst.net.nz
Sun Oct 28 21:25:11 UTC 2018
These ones are slightly interesting in that they could be sources of indeterminacy
or destructor chain failure, rather than just raising exceptions in rarely travelled
paths.
Having said that, I doubt these solve any problems we have actually noticed.
CI is running at https://gitlab.com/samba-team/devel/samba/pipelines/34610741
and an earlier version of these patches (along with ~70 others) passed here:
https://gitlab.com/samba-team/devel/samba/pipelines/34561952
cheers,
Douglas
-------------- next part --------------
From 64853820dd7f54ee32f261b3cdb01b821ec50b20 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 25 Oct 2018 22:09:59 +1300
Subject: [PATCH 1/2] python/samdb: properly use property()
Python's property() function works like this:
property([getter[, setter[, delete[, doc]]]])
but we have been forgetting the delete function, or rather setting it
to be a string. A string is not callable and is unlikely to succeed at
deleting the property.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
python/samba/samdb.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/python/samba/samdb.py b/python/samba/samdb.py
index fa3e4b1b334..1abd94b1938 100644
--- a/python/samba/samdb.py
+++ b/python/samba/samdb.py
@@ -676,7 +676,7 @@ accountExpires: %u
return dsdb._samdb_get_domain_sid(self)
domain_sid = property(get_domain_sid, set_domain_sid,
- "SID for the domain")
+ doc="SID for the domain")
def set_invocation_id(self, invocation_id):
"""Set the invocation id for this SamDB handle.
@@ -690,7 +690,7 @@ accountExpires: %u
return dsdb._samdb_ntds_invocation_id(self)
invocation_id = property(get_invocation_id, set_invocation_id,
- "Invocation ID GUID")
+ doc="Invocation ID GUID")
def get_oid_from_attid(self, attid):
return dsdb._dsdb_get_oid_from_attid(self, attid)
--
2.11.0
From eff730365d78103ea5830b46b9c09d32af7b4d0d Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri, 26 Oct 2018 20:25:59 +1300
Subject: [PATCH 2/2] python: do not use "is" for string equality
This is not always going to work, and is not guaranteed to be
consistent even between minor versions.
Here is a simple counterexample:
>>> a = 'hello'
>>> a is 'hello'
True
>>> a is 'hello'.lower()
False
>>> a == a.lower()
True
Possibly it always works for the empty string, but we cannot rely
on that.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
python/samba/netcmd/domain.py | 4 ++--
python/samba/netcmd/user.py | 10 +++++-----
python/samba/provision/backend.py | 2 +-
python/samba/samdb.py | 4 ++--
source4/dsdb/tests/python/password_lockout.py | 6 +++---
5 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/python/samba/netcmd/domain.py b/python/samba/netcmd/domain.py
index a85c16a179b..159b5711d1e 100644
--- a/python/samba/netcmd/domain.py
+++ b/python/samba/netcmd/domain.py
@@ -2334,7 +2334,7 @@ class cmd_domain_trust_create(DomainTrustCommand):
def get_password(name):
password = None
while True:
- if password is not None and password is not '':
+ if password is not None and password != '':
return password
password = getpass("New %s Password: " % name)
passwordverify = getpass("Retype %s Password: " % name)
@@ -2625,7 +2625,7 @@ class cmd_domain_trust_create(DomainTrustCommand):
self.outf.write("Deleting local TDO.\n")
local_lsa.DeleteObject(local_tdo_handle)
local_tdo_handle = None
- if current_request['location'] is "remote":
+ if current_request['location'] == "remote":
raise self.RemoteRuntimeError(self, error, "%s" % (
current_request['name']))
raise self.LocalRuntimeError(self, error, "%s" % (
diff --git a/python/samba/netcmd/user.py b/python/samba/netcmd/user.py
index 87726d4636e..2b1410bd3ba 100644
--- a/python/samba/netcmd/user.py
+++ b/python/samba/netcmd/user.py
@@ -325,7 +325,7 @@ Example5 shows how to create an RFC2307/NIS domain enabled user account. If
smartcard_required=False):
if smartcard_required:
- if password is not None and password is not '':
+ if password is not None and password != '':
raise CommandError('It is not allowed to specify '
'--newpassword '
'together with --smartcard-required.')
@@ -340,7 +340,7 @@ Example5 shows how to create an RFC2307/NIS domain enabled user account. If
while True:
if smartcard_required:
break
- if password is not None and password is not '':
+ if password is not None and password != '':
break
password = getpass("New Password: ")
passwordverify = getpass("Retype Password: ")
@@ -714,7 +714,7 @@ class cmd_user_password(Command):
password = newpassword
while True:
- if password is not None and password is not '':
+ if password is not None and password != '':
break
password = getpass("New Password: ")
passwordverify = getpass("Retype Password: ")
@@ -798,7 +798,7 @@ Example3 shows how an administrator would reset TestUser3 user's password to pas
password = newpassword
if smartcard_required:
- if password is not None and password is not '':
+ if password is not None and password != '':
raise CommandError('It is not allowed to specify '
'--newpassword '
'together with --smartcard-required.')
@@ -817,7 +817,7 @@ Example3 shows how an administrator would reset TestUser3 user's password to pas
while True:
if smartcard_required:
break
- if password is not None and password is not '':
+ if password is not None and password != '':
break
password = getpass("New Password: ")
passwordverify = getpass("Retype Password: ")
diff --git a/python/samba/provision/backend.py b/python/samba/provision/backend.py
index 2e3b1dc0c0b..502afdf53ff 100644
--- a/python/samba/provision/backend.py
+++ b/python/samba/provision/backend.py
@@ -569,7 +569,7 @@ class OpenLDAPBackend(LDAPBackend):
self.slapd_provision_command.extend([self.ldap_uri, "-d0"])
uris = self.ldap_uri
- if server_port_string is not "":
+ if server_port_string != "":
uris = uris + " " + server_port_string
self.slapd_command.append(uris)
diff --git a/python/samba/samdb.py b/python/samba/samdb.py
index 1abd94b1938..415142b2245 100644
--- a/python/samba/samdb.py
+++ b/python/samba/samdb.py
@@ -380,7 +380,7 @@ member: %s
displayname += ' %s' % surname
cn = username
- if useusernameascn is None and displayname is not "":
+ if useusernameascn is None and displayname != "":
cn = displayname
user_dn = "CN=%s,%s,%s" % (cn, (userou or "CN=Users"), self.domain_dn())
@@ -405,7 +405,7 @@ member: %s
if givenname is not None:
ldbmessage["givenName"] = givenname
- if displayname is not "":
+ if displayname != "":
ldbmessage["displayName"] = displayname
ldbmessage["name"] = displayname
diff --git a/source4/dsdb/tests/python/password_lockout.py b/source4/dsdb/tests/python/password_lockout.py
index aceebd66072..14cf00adb90 100755
--- a/source4/dsdb/tests/python/password_lockout.py
+++ b/source4/dsdb/tests/python/password_lockout.py
@@ -151,11 +151,11 @@ userAccountControl: %d
""" % uac)
def _reset_by_method(self, res, method):
- if method is "ldap_userAccountControl":
+ if method == "ldap_userAccountControl":
self._reset_ldap_userAccountControl(res)
- elif method is "ldap_lockoutTime":
+ elif method == "ldap_lockoutTime":
self._reset_ldap_lockoutTime(res)
- elif method is "samr":
+ elif method == "samr":
self._reset_samr(res)
else:
self.assertTrue(False, msg="Invalid reset method[%s]" % method)
--
2.11.0
More information about the samba-technical
mailing list