[PATCH] initialise optional parameters in Python-C bindings

Douglas Bagnall douglas.bagnall at catalyst.net.nz
Thu Apr 12 06:11:43 UTC 2018


I noticed today how easy it is to leave a variable uninitialised using
the PyArg_ParseTuple* functions with the "|" token, and how disguised
this unsetness is hidden from both static checkers and people's eyes.
The "|" means all the rest of the arguments are optional and the
corresponding C variables won't be changed if they are omitted.

I applied some grep and found the following.

Douglas
-------------- next part --------------
From 071a6865751ee91045fd8b16d3e3155d03d41b87 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 12 Apr 2018 17:07:38 +1200
Subject: [PATCH 1/6] ldb/pyldb: initialize optional parameter in ldb_connect()

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 lib/ldb/pyldb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index 4b02edbd9d2..67ecafbc420 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -1147,21 +1147,21 @@ static PyObject *py_ldb_new(PyTypeObject *type, PyObject *args, PyObject *kwargs
 		PyErr_NoMemory();
 		return NULL;
 	}
 
 	ret->ldb_ctx = ldb;
 	return (PyObject *)ret;
 }
 
 static PyObject *py_ldb_connect(PyLdbObject *self, PyObject *args, PyObject *kwargs)
 {
-	char *url;
+	char *url = NULL;
 	unsigned int flags = 0;
 	PyObject *py_options = Py_None;
 	int ret;
 	const char **options;
 	const char * const kwnames[] = { "url", "flags", "options", NULL };
 	struct ldb_context *ldb_ctx;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|zIO",
 					 discard_const_p(char *, kwnames),
 					 &url, &flags, &py_options))
-- 
2.14.1


From 3cac1abee3c1de75481c983a37ccd31ce604eb52 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 12 Apr 2018 17:09:45 +1200
Subject: [PATCH 2/6] nbt/pynbt: initialize optional parameter in
 nbt_name_register

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 libcli/nbt/pynbt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libcli/nbt/pynbt.c b/libcli/nbt/pynbt.c
index 6337092b226..bee98b3b5cb 100644
--- a/libcli/nbt/pynbt.c
+++ b/libcli/nbt/pynbt.c
@@ -258,20 +258,21 @@ static PyObject *py_nbt_name_register(PyObject *self, PyObject *args, PyObject *
 	PyObject *ret, *py_dest, *py_name;
 	struct nbt_name_register io;
 	NTSTATUS status;
 
 	const char *kwnames[] = { "name", "address", "dest", "register_demand", "broadcast",
 		                  "multi_homed", "ttl", "timeout", "retries", NULL };
 
 	io.in.broadcast = true;
 	io.in.multi_homed = true;
 	io.in.register_demand = true;
+	io.in.ttl = 0;
 	io.in.timeout = 0;
 	io.in.retries = 0;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "OsO|bbbiii:query_name",
 					 discard_const_p(char *, kwnames),
 					 &py_name, &io.in.address, &py_dest,
 					 &io.in.register_demand,
 					 &io.in.broadcast, &io.in.multi_homed,
 					 &io.in.ttl, &io.in.timeout, &io.in.retries)) {
 		return NULL;
-- 
2.14.1


From 895669cefdbf2a993b53cba99c2abfee05d5afea Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 12 Apr 2018 17:10:10 +1200
Subject: [PATCH 3/6] nbt/pynbt: initialize optional parameter in
 nbt_name_refresh

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 libcli/nbt/pynbt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libcli/nbt/pynbt.c b/libcli/nbt/pynbt.c
index bee98b3b5cb..254c98a4b89 100644
--- a/libcli/nbt/pynbt.c
+++ b/libcli/nbt/pynbt.c
@@ -314,20 +314,21 @@ static PyObject *py_nbt_name_refresh(PyObject *self, PyObject *args, PyObject *k
 	nbt_node_Object *node = (nbt_node_Object *)self;
 	PyObject *ret, *py_dest, *py_name;
 	struct nbt_name_refresh io;
 	NTSTATUS status;
 
 	const char *kwnames[] = { "name", "address", "dest", "nb_flags", "broadcast",
 		                  "ttl", "timeout", "retries", NULL };
 
 	io.in.broadcast = true;
 	io.in.nb_flags = 0;
+	io.in.ttl = 0;
 	io.in.timeout = 0;
 	io.in.retries = 0;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "OsO|ibiii:query_name",
 					 discard_const_p(char *, kwnames),
 					 &py_name, &io.in.address, &py_dest,
 					 &io.in.nb_flags,
 					 &io.in.broadcast,
 					 &io.in.ttl, &io.in.timeout, &io.in.retries)) {
 		return NULL;
-- 
2.14.1


From abd3a8d721f1321024592b368363c48a8c2f19a7 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 12 Apr 2018 17:13:05 +1200
Subject: [PATCH 4/6] s3/py_passdb: initialize optional parameters earlier

It is just a bit easier to see what is happening.


Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 source3/passdb/py_passdb.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/source3/passdb/py_passdb.c b/source3/passdb/py_passdb.c
index bb0952fd1ea..31e3907c8bc 100644
--- a/source3/passdb/py_passdb.c
+++ b/source3/passdb/py_passdb.c
@@ -1900,29 +1900,28 @@ static PyObject *py_pdb_delete_group_mapping_entry(PyObject *self, PyObject *arg
 
 
 static PyObject *py_pdb_enum_group_mapping(PyObject *self, PyObject *args)
 {
 	TALLOC_CTX *frame = talloc_stackframe();
 	NTSTATUS status;
 	struct pdb_methods *methods;
 	enum lsa_SidType sid_name_use;
 	int lsa_sidtype_value = SID_NAME_UNKNOWN;
 	int unix_only = 0;
-	PyObject *py_domain_sid;
+	PyObject *py_domain_sid = Py_None;
 	struct dom_sid *domain_sid = NULL;
 	GROUP_MAP **gmap = NULL;
 	GROUP_MAP *group_map;
 	size_t num_entries;
 	PyObject *py_gmap_list, *py_group_map;
 	int i;
 
-	py_domain_sid = Py_None;
 	Py_INCREF(Py_None);
 
 	if (!PyArg_ParseTuple(args, "|O!ii:enum_group_mapping", dom_sid_Type, &py_domain_sid,
 					&lsa_sidtype_value, &unix_only)) {
 		talloc_free(frame);
 		return NULL;
 	}
 
 	methods = pytalloc_get_ptr(self);
 
@@ -2595,24 +2594,23 @@ static PyObject *py_pdb_search_groups(PyObject *self, PyObject *unused)
 }
 
 
 static PyObject *py_pdb_search_aliases(PyObject *self, PyObject *args)
 {
 	TALLOC_CTX *frame = talloc_stackframe();
 	struct pdb_methods *methods;
 	struct pdb_search *search;
 	struct samr_displayentry *entry;
 	PyObject *py_aliaslist, *py_dict;
-	PyObject *py_domain_sid;
+	PyObject *py_domain_sid = Py_None;
 	struct dom_sid *domain_sid = NULL;
 
-	py_domain_sid = Py_None;
 	Py_INCREF(Py_None);
 
 	if (!PyArg_ParseTuple(args, "|O!:search_aliases", dom_sid_Type, &py_domain_sid)) {
 		talloc_free(frame);
 		return NULL;
 	}
 
 	methods = pytalloc_get_ptr(self);
 
 	if (py_domain_sid != Py_None) {
-- 
2.14.1


From a44ed397915ae6a60ee96d696b979c65a0a6cc6d Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 12 Apr 2018 17:15:19 +1200
Subject: [PATCH 5/6] s4/lib/py-registry: initialize optional parameters for
 open_* functions

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 source4/lib/registry/pyregistry.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/source4/lib/registry/pyregistry.c b/source4/lib/registry/pyregistry.c
index f36973feb35..5d7f7f212b8 100644
--- a/source4/lib/registry/pyregistry.c
+++ b/source4/lib/registry/pyregistry.c
@@ -243,21 +243,23 @@ static PyMethodDef hive_key_methods[] = {
 
 static PyObject *hive_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) {
 	Py_RETURN_NONE;
 }
 
 static PyObject *py_open_hive(PyTypeObject *type, PyObject *args, PyObject *kwargs)
 {
 	const char *kwnames[] = { "location", "lp_ctx", "session_info", "credentials", NULL };
 	WERROR result;
 	struct loadparm_context *lp_ctx;
-	PyObject *py_lp_ctx, *py_session_info, *py_credentials;
+	PyObject *py_lp_ctx = Py_None;
+	PyObject *py_session_info = Py_None;
+	PyObject *py_credentials = Py_None;
 	struct auth_session_info *session_info;
 	struct cli_credentials *credentials;
 	char *location;
 	struct hive_key *hive_key;
 	TALLOC_CTX *mem_ctx;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "s|OOO",
 					 discard_const_p(char *, kwnames),
 	                                 &location,
 					 &py_lp_ctx, &py_session_info,
@@ -305,21 +307,23 @@ PyTypeObject PyRegistryKey = {
 	.tp_name = "RegistryKey",
 	.tp_flags = Py_TPFLAGS_DEFAULT,
 };
 
 static PyObject *py_open_samba(PyObject *self, PyObject *args, PyObject *kwargs)
 {
 	const char *kwnames[] = { "lp_ctx", "session_info", NULL };
 	struct registry_context *reg_ctx;
 	WERROR result;
 	struct loadparm_context *lp_ctx;
-	PyObject *py_lp_ctx, *py_session_info, *py_credentials;
+	PyObject *py_lp_ctx = Py_None;
+	PyObject *py_session_info = Py_None;
+	PyObject *py_credentials = Py_None;
 	struct auth_session_info *session_info;
 	struct cli_credentials *credentials;
 	TALLOC_CTX *mem_ctx;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOO",
 					 discard_const_p(char *, kwnames),
 					 &py_lp_ctx, &py_session_info,
 					 &py_credentials))
 		return NULL;
 
-- 
2.14.1


From 881fa252dd954ce855484b2496aa5ae1efebf059 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 12 Apr 2018 17:19:20 +1200
Subject: [PATCH 6/6] s4/webserver: initialise optional parameter

OK, this is unused and unimplemented.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 source4/web_server/wsgi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/web_server/wsgi.c b/source4/web_server/wsgi.c
index 0b1c5d29014..0e31dade970 100644
--- a/source4/web_server/wsgi.c
+++ b/source4/web_server/wsgi.c
@@ -197,21 +197,21 @@ static PyObject *py_input_readline(PyObject *_self)
 {
 	/* FIXME */
 	PyErr_SetString(PyExc_NotImplementedError, 
 			"readline() not yet implemented");
 	return NULL;
 }
 
 static PyObject *py_input_readlines(PyObject *_self, PyObject *args, PyObject *kwargs)
 {
 	const char *kwnames[] = { "hint", NULL };
-	int hint;
+	int hint = 0;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|i", discard_const_p(char *, kwnames), &hint))
 		return NULL;
 	
 	/* FIXME */
 	PyErr_SetString(PyExc_NotImplementedError, 
 			"readlines() not yet implemented");
 	return NULL;
 }
 
-- 
2.14.1



More information about the samba-technical mailing list