Patches to for type consistency in Python pidl

Douglas Bagnall douglas.bagnall at catalyst.net.nz
Thu Jun 25 00:20:33 MDT 2015


hello,

I am not entirely comfortable with the third patch which treats time_t
as uint32_t. As far as I can tell, time_t is very rarely uint32_t but
that is how it is defined in ndr_basic.c.

I don't have any tests yet, and I think there are a few more of these to
do, but I thiught I'd send this out for comment before I went too far.

cheers,
Douglas
-------------- next part --------------
From 9c6661748f75d8aaf00b6a1a0ec1aff3a7a63b89 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 18 Jun 2015 12:38:22 +1200
Subject: [PATCH 1/6] Treat uid_t, gid_t as 64 bit in Pidl Python bindings

This follows their treatment in librpc/ndr/ndr_basic.c.


Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 pidl/lib/Parse/Pidl/Samba4/Python.pm | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/pidl/lib/Parse/Pidl/Samba4/Python.pm b/pidl/lib/Parse/Pidl/Samba4/Python.pm
index 0f54944..ff68e7a 100644
--- a/pidl/lib/Parse/Pidl/Samba4/Python.pm
+++ b/pidl/lib/Parse/Pidl/Samba4/Python.pm
@@ -972,7 +972,9 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 		return;
 	}
 	if ($actual_ctype->{TYPE} eq "SCALAR" ) {
-		if (expandAlias($actual_ctype->{NAME}) =~ /^(u?int64|hyper|dlong|udlong|udlongr|NTTIME_hyper|NTTIME|NTTIME_1sec)$/) {
+		if (expandAlias($actual_ctype->{NAME}) =~ /^(u?int64|hyper|dlong|udlong|udlongr
+                                                           |NTTIME_hyper|NTTIME|NTTIME_1sec
+                                                           |uid_t|gid_t)$/x) {
 			$self->pidl("if (PyLong_Check($cvar)) {");
 			$self->indent;
 			$self->pidl("$target = PyLong_AsLongLong($cvar);");
@@ -990,7 +992,7 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 			$self->pidl("}");
 			return;
 		}
-		if (expandAlias($actual_ctype->{NAME}) =~ /^(char|u?int[0-9]*|time_t|uid_t|gid_t)$/) {
+		if (expandAlias($actual_ctype->{NAME}) =~ /^(char|u?int[0-9]*|time_t)$/) {
 			$self->pidl("PY_CHECK_TYPE(&PyInt_Type, $cvar, $fail);");
 			$self->pidl("$target = PyInt_AsLong($cvar);");
 			return;
@@ -1203,11 +1205,11 @@ sub ConvertScalarToPython($$$)
 		return "PyLong_FromLongLong($cvar)";
 	}
 
-	if ($ctypename =~ /^(uint64|hyper|udlong|udlongr|NTTIME_hyper|NTTIME|NTTIME_1sec)$/) {
+	if ($ctypename =~ /^(uint64|hyper|udlong|udlongr|NTTIME_hyper|NTTIME|NTTIME_1sec|uid_t|gid_t)$/) {
 		return "PyLong_FromUnsignedLongLong($cvar)";
 	}
 
-	if ($ctypename =~ /^(char|u?int[0-9]*|time_t|uid_t|gid_t)$/) {
+	if ($ctypename =~ /^(char|u?int[0-9]*|time_t)$/) {
 		return "PyInt_FromLong($cvar)";
 	}
 
-- 
1.9.1


From 2b7e3afc390d69eebc8564c45bc1033c0ce6aca7 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 18 Jun 2015 12:57:12 +1200
Subject: [PATCH 2/6] Treat uint32 as unsigned values in Pidl Python bindings

This slightly increases memory use as uint32 now maps to Python Long
rather than Python Int. On the other hand the values have a better
chance of being consistent.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 pidl/lib/Parse/Pidl/Samba4/Python.pm | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/pidl/lib/Parse/Pidl/Samba4/Python.pm b/pidl/lib/Parse/Pidl/Samba4/Python.pm
index ff68e7a..ff70318 100644
--- a/pidl/lib/Parse/Pidl/Samba4/Python.pm
+++ b/pidl/lib/Parse/Pidl/Samba4/Python.pm
@@ -992,6 +992,20 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 			$self->pidl("}");
 			return;
 		}
+		if (expandAlias($actual_ctype->{NAME}) =~ /^(uint32)$/x) {
+			$self->pidl("if (PyLong_Check($cvar) || PyInt_Check($cvar)) {");
+			$self->indent;
+			$self->pidl("$target = PyInt_AsUnsignedLongMask($cvar);");
+			$self->deindent;
+			$self->pidl("} else {");
+			$self->indent;
+			$self->pidl("PyErr_Format(PyExc_TypeError, \"Expected type %s or %s\",\\");
+			$self->pidl("  PyInt_Type.tp_name, PyLong_Type.tp_name);");
+			$self->pidl($fail);
+			$self->deindent;
+			$self->pidl("}");
+			return;
+                }
 		if (expandAlias($actual_ctype->{NAME}) =~ /^(char|u?int[0-9]*|time_t)$/) {
 			$self->pidl("PY_CHECK_TYPE(&PyInt_Type, $cvar, $fail);");
 			$self->pidl("$target = PyInt_AsLong($cvar);");
@@ -1209,6 +1223,10 @@ sub ConvertScalarToPython($$$)
 		return "PyLong_FromUnsignedLongLong($cvar)";
 	}
 
+	if ($ctypename =~ /^(uint32)$/) {
+		return "PyLong_FromUnsignedLong($cvar)";
+	}
+
 	if ($ctypename =~ /^(char|u?int[0-9]*|time_t)$/) {
 		return "PyInt_FromLong($cvar)";
 	}
-- 
1.9.1


From b75512330eae0f9078657403e11489e6185f04ce Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 18 Jun 2015 13:06:33 +1200
Subject: [PATCH 3/6] Python Pidl: keep C type name in a temp var

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 pidl/lib/Parse/Pidl/Samba4/Python.pm | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/pidl/lib/Parse/Pidl/Samba4/Python.pm b/pidl/lib/Parse/Pidl/Samba4/Python.pm
index ff70318..06eb6c2 100644
--- a/pidl/lib/Parse/Pidl/Samba4/Python.pm
+++ b/pidl/lib/Parse/Pidl/Samba4/Python.pm
@@ -972,9 +972,10 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 		return;
 	}
 	if ($actual_ctype->{TYPE} eq "SCALAR" ) {
-		if (expandAlias($actual_ctype->{NAME}) =~ /^(u?int64|hyper|dlong|udlong|udlongr
-                                                           |NTTIME_hyper|NTTIME|NTTIME_1sec
-                                                           |uid_t|gid_t)$/x) {
+                my $ctypename = expandAlias($actual_ctype->{NAME});
+		if ($ctypename =~ /^(u?int64|hyper|dlong|udlong|udlongr
+                                   |NTTIME_hyper|NTTIME|NTTIME_1sec
+                                   |uid_t|gid_t)$/x) {
 			$self->pidl("if (PyLong_Check($cvar)) {");
 			$self->indent;
 			$self->pidl("$target = PyLong_AsLongLong($cvar);");
@@ -992,7 +993,7 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 			$self->pidl("}");
 			return;
 		}
-		if (expandAlias($actual_ctype->{NAME}) =~ /^(uint32)$/x) {
+		if ($ctypename =~ /^(uint32)$/x) {
 			$self->pidl("if (PyLong_Check($cvar) || PyInt_Check($cvar)) {");
 			$self->indent;
 			$self->pidl("$target = PyInt_AsUnsignedLongMask($cvar);");
@@ -1006,7 +1007,7 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 			$self->pidl("}");
 			return;
                 }
-		if (expandAlias($actual_ctype->{NAME}) =~ /^(char|u?int[0-9]*|time_t)$/) {
+		if ($ctypename =~ /^(char|u?int[0-9]*|time_t)$/) {
 			$self->pidl("PY_CHECK_TYPE(&PyInt_Type, $cvar, $fail);");
 			$self->pidl("$target = PyInt_AsLong($cvar);");
 			return;
-- 
1.9.1


From 4fb5a10a4501ffb1410f14adbe850238e1a07c02 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 18 Jun 2015 13:20:59 +1200
Subject: [PATCH 4/6] Treat time_t as uint32_t in Python Pidl, following NDR

This is how we define time_t, so we should use it.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 pidl/lib/Parse/Pidl/Samba4/Python.pm | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/pidl/lib/Parse/Pidl/Samba4/Python.pm b/pidl/lib/Parse/Pidl/Samba4/Python.pm
index 06eb6c2..d3c3475 100644
--- a/pidl/lib/Parse/Pidl/Samba4/Python.pm
+++ b/pidl/lib/Parse/Pidl/Samba4/Python.pm
@@ -993,7 +993,7 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 			$self->pidl("}");
 			return;
 		}
-		if ($ctypename =~ /^(uint32)$/x) {
+		if ($ctypename =~ /^(uint32|time_t)$/x) {
 			$self->pidl("if (PyLong_Check($cvar) || PyInt_Check($cvar)) {");
 			$self->indent;
 			$self->pidl("$target = PyInt_AsUnsignedLongMask($cvar);");
@@ -1007,7 +1007,7 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 			$self->pidl("}");
 			return;
                 }
-		if ($ctypename =~ /^(char|u?int[0-9]*|time_t)$/) {
+		if ($ctypename =~ /^(char|u?int[0-9]*)$/) {
 			$self->pidl("PY_CHECK_TYPE(&PyInt_Type, $cvar, $fail);");
 			$self->pidl("$target = PyInt_AsLong($cvar);");
 			return;
@@ -1224,11 +1224,14 @@ sub ConvertScalarToPython($$$)
 		return "PyLong_FromUnsignedLongLong($cvar)";
 	}
 
-	if ($ctypename =~ /^(uint32)$/) {
+	# In reality time_t is a mess, usually 32 bit signed,
+	# sometimes 64 bit, while C11 allows it to be floating point.
+	# In NDR we treat it as uint32_t.
+	if ($ctypename =~ /^(uint32|time_t)$/) {
 		return "PyLong_FromUnsignedLong($cvar)";
 	}
 
-	if ($ctypename =~ /^(char|u?int[0-9]*|time_t)$/) {
+	if ($ctypename =~ /^(char|u?int[0-9]*)$/) {
 		return "PyInt_FromLong($cvar)";
 	}
 
-- 
1.9.1


From 885ad7cc4e4ce0362a349e2a34ed03239cdc1581 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 18 Jun 2015 13:46:46 +1200
Subject: [PATCH 5/6] Python Pidl: properly convert unsigned 64 bit objects

This is being a bit conservative with the handling of Python objects.
The conversion function PyInt_AsUnsignedLongLongMask() will coerce
objects (presumably floats) into ints/longs if it can, but in keeping
with existing practice we reject those non-ints/longs before it sees
them.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 pidl/lib/Parse/Pidl/Samba4/Python.pm | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/pidl/lib/Parse/Pidl/Samba4/Python.pm b/pidl/lib/Parse/Pidl/Samba4/Python.pm
index d3c3475..5ad1fa6 100644
--- a/pidl/lib/Parse/Pidl/Samba4/Python.pm
+++ b/pidl/lib/Parse/Pidl/Samba4/Python.pm
@@ -972,10 +972,8 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 		return;
 	}
 	if ($actual_ctype->{TYPE} eq "SCALAR" ) {
-                my $ctypename = expandAlias($actual_ctype->{NAME});
-		if ($ctypename =~ /^(u?int64|hyper|dlong|udlong|udlongr
-                                   |NTTIME_hyper|NTTIME|NTTIME_1sec
-                                   |uid_t|gid_t)$/x) {
+		my $ctypename = expandAlias($actual_ctype->{NAME});
+		if ($ctypename =~ /^(int64|hyper|dlong)$/) {
 			$self->pidl("if (PyLong_Check($cvar)) {");
 			$self->indent;
 			$self->pidl("$target = PyLong_AsLongLong($cvar);");
@@ -993,6 +991,21 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 			$self->pidl("}");
 			return;
 		}
+		if ($ctypename =~ /^(uint64|udlong|udlongr|NTTIME_hyper
+				   |NTTIME|NTTIME_1sec|uid_t|gid_t)$/x) {
+			$self->pidl("if (PyLong_Check($cvar) || PyInt_Check($cvar)) {");
+			$self->indent;
+			$self->pidl("$target = PyInt_AsUnsignedLongLongMask($cvar);");
+			$self->deindent;
+			$self->pidl("} else {");
+			$self->indent;
+			$self->pidl("PyErr_Format(PyExc_TypeError, \"Expected type %s or %s\",\\");
+			$self->pidl("  PyInt_Type.tp_name, PyLong_Type.tp_name);");
+			$self->pidl($fail);
+			$self->deindent;
+			$self->pidl("}");
+			return;
+		}
 		if ($ctypename =~ /^(uint32|time_t)$/x) {
 			$self->pidl("if (PyLong_Check($cvar) || PyInt_Check($cvar)) {");
 			$self->indent;
-- 
1.9.1


From 8074e9a3103718c3b147af3962b2c0e03cb8d6fe Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 18 Jun 2015 13:51:56 +1200
Subject: [PATCH 6/6] Python Pidl: treat u?int3264 values as possibly 64 bit

It seems these usually not 64 bit, but it should be harmless to treat
them as if they are.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 pidl/lib/Parse/Pidl/Samba4/Python.pm | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/pidl/lib/Parse/Pidl/Samba4/Python.pm b/pidl/lib/Parse/Pidl/Samba4/Python.pm
index 5ad1fa6..3ca732f 100644
--- a/pidl/lib/Parse/Pidl/Samba4/Python.pm
+++ b/pidl/lib/Parse/Pidl/Samba4/Python.pm
@@ -973,7 +973,7 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 	}
 	if ($actual_ctype->{TYPE} eq "SCALAR" ) {
 		my $ctypename = expandAlias($actual_ctype->{NAME});
-		if ($ctypename =~ /^(int64|hyper|dlong)$/) {
+		if ($ctypename =~ /^(int64|int3264|hyper|dlong)$/) {
 			$self->pidl("if (PyLong_Check($cvar)) {");
 			$self->indent;
 			$self->pidl("$target = PyLong_AsLongLong($cvar);");
@@ -991,7 +991,7 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 			$self->pidl("}");
 			return;
 		}
-		if ($ctypename =~ /^(uint64|udlong|udlongr|NTTIME_hyper
+		if ($ctypename =~ /^(uint64|uint3264|udlong|udlongr|NTTIME_hyper
 				   |NTTIME|NTTIME_1sec|uid_t|gid_t)$/x) {
 			$self->pidl("if (PyLong_Check($cvar) || PyInt_Check($cvar)) {");
 			$self->indent;
@@ -1019,7 +1019,7 @@ sub ConvertObjectFromPythonData($$$$$$;$)
 			$self->deindent;
 			$self->pidl("}");
 			return;
-                }
+		}
 		if ($ctypename =~ /^(char|u?int[0-9]*)$/) {
 			$self->pidl("PY_CHECK_TYPE(&PyInt_Type, $cvar, $fail);");
 			$self->pidl("$target = PyInt_AsLong($cvar);");
@@ -1229,11 +1229,11 @@ sub ConvertScalarToPython($$$)
 
 	$ctypename = expandAlias($ctypename);
 
-	if ($ctypename =~ /^(int64|dlong)$/) {
+	if ($ctypename =~ /^(int64|dlong|int3264)$/) {
 		return "PyLong_FromLongLong($cvar)";
 	}
 
-	if ($ctypename =~ /^(uint64|hyper|udlong|udlongr|NTTIME_hyper|NTTIME|NTTIME_1sec|uid_t|gid_t)$/) {
+	if ($ctypename =~ /^(uint64|hyper|udlong|udlongr|NTTIME_hyper|NTTIME|NTTIME_1sec|uid_t|gid_t|uint3264)$/) {
 		return "PyLong_FromUnsignedLongLong($cvar)";
 	}
 
-- 
1.9.1


More information about the samba-technical mailing list