pytalloc patches

Petr Viktorin pviktori at redhat.com
Tue Apr 14 07:47:26 MDT 2015


On 04/14/2015 12:30 PM, Jelmer Vernooij wrote:
> On Tue, Apr 14, 2015 at 08:24:37AM +0200, Andreas Schneider wrote:
>> On Monday 13 April 2015 11:52:42 Andreas Schneider wrote:
>>> On Friday 10 April 2015 10:16:52 Petr Viktorin wrote:
>>>> On 04/06/2015 04:40 AM, Jelmer Vernooij wrote:
>>>>> These are the patches from Petr with my Reviewed-By added. Please also
>>>>> review & push.
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Jelmer
>>>>
>>>> Thank you for the review, Jelmer.
>>>>
>>>> Can anyone else find time to do a second review?
>>>
>>> Pushed to autobuild, thanks for your contribution Petr!
>>>
>>
>> The pytalloc test fails with these patches:
>>
>> https://git.samba.org/asn/samba-autobuild/talloc.stderr
>>
>> Please fix them and resubmit the patchset.
>
> Ah, looks like 2.6 compatibility. :(

Indeed, my bad. I've spent too much time in 2.7 land.

Here are fixed patches, tested with Python 2.6 as well.


-- 
Petr Viktorin
-------------- next part --------------
From 8ec74dcdfe2bba148370d50b0d3738e5e1d25cc3 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori at redhat.com>
Date: Fri, 6 Mar 2015 18:57:00 +0100
Subject: [PATCH 01/15] pytalloc: Fix comparison of disparate types

When fed Python objects of different types, pytalloc_default_cmp
compared pointers to PyType objects that weren't part of an array,
resulting in undefined behavior.

This makes things a bit better (though it still casts ptrdiff_t to int).

Reviewed-By: Jelmer Vernooij <jelmer at samba.org>
Signed-off-by: Petr Viktorin <pviktori at redhat.com>
---
 lib/talloc/pytalloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/talloc/pytalloc.c b/lib/talloc/pytalloc.c
index 80196c6..ac4fe0f 100644
--- a/lib/talloc/pytalloc.c
+++ b/lib/talloc/pytalloc.c
@@ -102,7 +102,7 @@ static int pytalloc_default_cmp(PyObject *_obj1, PyObject *_obj2)
 	pytalloc_Object *obj1 = (pytalloc_Object *)_obj1,
 					 *obj2 = (pytalloc_Object *)_obj2;
 	if (obj1->ob_type != obj2->ob_type)
-		return (obj1->ob_type - obj2->ob_type);
+		return ((char *)obj1->ob_type - (char *)obj2->ob_type);
 
 	return ((char *)pytalloc_get_ptr(obj1) - (char *)pytalloc_get_ptr(obj2));
 }
-- 
2.1.0


From 420d62bacb1d90da856369c44d2dea8710fbe796 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori at redhat.com>
Date: Fri, 6 Mar 2015 11:55:49 +0100
Subject: [PATCH 02/15] wafsamba: Add install argument to SAMBA_PYTHON

This allows building Python modules that are only used for testing.

Reviewed-By: Jelmer Vernooij <jelmer at samba.org>
Signed-off-by: Petr Viktorin <pviktori at redhat.com>
---
 buildtools/wafsamba/samba_python.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/buildtools/wafsamba/samba_python.py b/buildtools/wafsamba/samba_python.py
index 1ec2f7b..a371b43 100644
--- a/buildtools/wafsamba/samba_python.py
+++ b/buildtools/wafsamba/samba_python.py
@@ -34,6 +34,7 @@ def SAMBA_PYTHON(bld, name,
                  init_function_sentinel=None,
                  local_include=True,
                  vars=None,
+                 install=True,
                  enabled=True):
     '''build a python extension for Samba'''
 
@@ -64,6 +65,7 @@ def SAMBA_PYTHON(bld, name,
                       install_path='${PYTHONARCHDIR}',
                       allow_undefined_symbols=True,
                       allow_warnings=True,
+                      install=install,
                       enabled=enabled)
 
 Build.BuildContext.SAMBA_PYTHON = SAMBA_PYTHON
-- 
2.1.0


From 4c63e6afd75b7bfc31c2ab6e48486892664ab56e Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori at redhat.com>
Date: Thu, 5 Mar 2015 10:06:05 +0100
Subject: [PATCH 03/15] pytalloc: Add tests

Add tests for pytalloc.

Since talloc objects can't be created from Python, a C extension
with helpers is added.

Signed-off-by: Petr Viktorin <pviktori at redhat.com>
---
 lib/talloc/test_pytalloc.c  | 128 ++++++++++++++++++++++++++++++++++++++++++++
 lib/talloc/test_pytalloc.py | 114 +++++++++++++++++++++++++++++++++++++++
 lib/talloc/wscript          |  19 ++++++-
 3 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 lib/talloc/test_pytalloc.c
 create mode 100644 lib/talloc/test_pytalloc.py

diff --git a/lib/talloc/test_pytalloc.c b/lib/talloc/test_pytalloc.c
new file mode 100644
index 0000000..2eaa7c3
--- /dev/null
+++ b/lib/talloc/test_pytalloc.c
@@ -0,0 +1,128 @@
+/*
+   Samba Unix SMB/CIFS implementation.
+
+   C utilities for the pytalloc test suite.
+   Provides the "_test_pytalloc" Python module.
+
+   NOTE: Please read talloc_guide.txt for full documentation
+
+   Copyright (C) Petr Viktorin 2015
+
+     ** NOTE! The following LGPL license applies to the talloc
+     ** library. This does NOT imply that all of Samba is released
+     ** under the LGPL
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 3 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <Python.h>
+#include <talloc.h>
+#include <pytalloc.h>
+
+static PyObject *testpytalloc_new(PyTypeObject *mod)
+{
+	char *obj = talloc_strdup(NULL, "This is a test string");;
+	return pytalloc_steal(pytalloc_GetObjectType(), obj);
+}
+
+static PyObject *testpytalloc_get_object_type(PyObject *mod) {
+	PyObject *type = (PyObject *)pytalloc_GetObjectType();
+	Py_INCREF(type);
+	return type;
+}
+
+static PyObject *testpytalloc_reference(PyObject *mod, PyObject *args) {
+	pytalloc_Object *source = NULL;
+	void *ptr;
+
+	if (!PyArg_ParseTuple(args, "O!", pytalloc_GetObjectType(), &source))
+		return NULL;
+
+	ptr = source->ptr;
+	return pytalloc_reference_ex(pytalloc_GetObjectType(), ptr, ptr);
+}
+
+static PyMethodDef test_talloc_methods[] = {
+	{ "new", (PyCFunction)testpytalloc_new, METH_NOARGS,
+		"create a talloc Object with a testing string"},
+	{ "get_object_type", (PyCFunction)testpytalloc_get_object_type, METH_NOARGS,
+		"call pytalloc_GetObjectType"},
+	{ "reference", (PyCFunction)testpytalloc_reference, METH_VARARGS,
+		"call pytalloc_reference_ex"},
+	{ NULL }
+};
+
+static PyTypeObject DObject_Type;
+
+static int dobject_destructor(void *ptr)
+{
+	PyObject *destructor_func = *talloc_get_type(ptr, PyObject*);
+	PyObject *ret;
+	ret = PyObject_CallObject(destructor_func, NULL);
+	Py_DECREF(destructor_func);
+	if (ret == NULL) {
+		PyErr_Print();
+	} else {
+		Py_DECREF(ret);
+	}
+	return 0;
+}
+
+static PyObject *dobject_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
+{
+	PyObject *destructor_func = NULL;
+	PyObject **obj;
+
+	if (!PyArg_ParseTuple(args, "O", &destructor_func))
+		return NULL;
+	Py_INCREF(destructor_func);
+
+	obj = talloc(NULL, PyObject*);
+	*obj = destructor_func;
+
+	talloc_set_destructor((void*)obj, dobject_destructor);
+	return pytalloc_steal(&DObject_Type, obj);
+}
+
+static PyTypeObject DObject_Type = {
+	.tp_name = "_test_pytalloc.DObject",
+	.tp_basicsize = sizeof(pytalloc_Object),
+	.tp_methods = NULL,
+	.tp_new = dobject_new,
+	.tp_flags = Py_TPFLAGS_DEFAULT,
+	.tp_doc = "test talloc object that calls a function when underlying data is freed\n",
+};
+
+#define MODULE_DOC "Test utility module for pytalloc"
+
+void init_test_pytalloc(void);
+void init_test_pytalloc(void)
+{
+	PyObject *m;
+
+	DObject_Type.tp_base = pytalloc_GetObjectType();
+	if (PyType_Ready(&DObject_Type) < 0) {
+		return;
+	}
+
+	m = Py_InitModule3("_test_pytalloc", test_talloc_methods, MODULE_DOC);
+
+	if (m == NULL) {
+		return;
+	}
+
+	Py_INCREF(&DObject_Type);
+	Py_INCREF(DObject_Type.tp_base);
+	PyModule_AddObject(m, "DObject", (PyObject *)&DObject_Type);
+}
diff --git a/lib/talloc/test_pytalloc.py b/lib/talloc/test_pytalloc.py
new file mode 100644
index 0000000..961bfcb
--- /dev/null
+++ b/lib/talloc/test_pytalloc.py
@@ -0,0 +1,114 @@
+#!/usr/bin/env python
+# Simple tests for the talloc python bindings.
+# Copyright (C) 2015 Petr Viktorin <pviktori at redhat.com>
+
+import unittest
+import subprocess
+import sys
+import re
+import gc
+
+import talloc
+import _test_pytalloc
+
+def dummy_func():
+    pass
+
+
+class TallocTests(unittest.TestCase):
+
+    def test_report_full(self):
+        # report_full is hardcoded to print to stdout, so use a subprocess
+        process = subprocess.Popen([
+            sys.executable, '-c',
+            """if True:
+            import talloc, _test_pytalloc
+            obj = _test_pytalloc.new()
+            talloc.report_full(obj)
+            """
+        ], stdout=subprocess.PIPE)
+        output, stderr = process.communicate()
+        output = str(output)
+        self.assertTrue("full talloc report on 'talloc.Object" in output)
+        self.assertTrue("This is a test string" in output)
+
+    def test_totalblocks(self):
+        obj = _test_pytalloc.new()
+        # Two blocks: the string, and the name
+        self.assertEqual(talloc.total_blocks(obj), 2)
+
+    def test_repr(self):
+        obj = _test_pytalloc.new()
+        prefix = '<talloc.Object talloc object at'
+        self.assertTrue(repr(obj).startswith(prefix))
+        self.assertEqual(repr(obj), str(obj))
+
+    def test_destructor(self):
+        # Check correct lifetime of the talloc'd data
+        lst = []
+        obj = _test_pytalloc.DObject(lambda: lst.append('dead'))
+        self.assertEqual(lst, [])
+        del obj
+        gc.collect()
+        self.assertEqual(lst, ['dead'])
+
+
+class TallocComparisonTests(unittest.TestCase):
+
+    def test_compare_same(self):
+        obj1 = _test_pytalloc.new()
+        self.assertTrue(obj1 == obj1)
+        self.assertFalse(obj1 != obj1)
+        self.assertTrue(obj1 <= obj1)
+        self.assertFalse(obj1 < obj1)
+        self.assertTrue(obj1 >= obj1)
+        self.assertFalse(obj1 > obj1)
+
+    def test_compare_different(self):
+        # object comparison is consistent
+        obj1, obj2 = sorted([
+            _test_pytalloc.new(),
+            _test_pytalloc.new()])
+        self.assertFalse(obj1 == obj2)
+        self.assertTrue(obj1 != obj2)
+        self.assertTrue(obj1 <= obj2)
+        self.assertTrue(obj1 < obj2)
+        self.assertFalse(obj1 >= obj2)
+        self.assertFalse(obj1 > obj2)
+
+    def test_compare_different_types(self):
+        # object comparison falls back to comparing types
+        if talloc.Object < _test_pytalloc.DObject:
+            obj1 = _test_pytalloc.new()
+            obj2 = _test_pytalloc.DObject(dummy_func)
+        else:
+            obj2 = _test_pytalloc.new()
+            obj1 = _test_pytalloc.DObject(dummy_func)
+        self.assertFalse(obj1 == obj2)
+        self.assertTrue(obj1 != obj2)
+        self.assertTrue(obj1 <= obj2)
+        self.assertTrue(obj1 < obj2)
+        self.assertFalse(obj1 >= obj2)
+        self.assertFalse(obj1 > obj2)
+
+
+class TallocUtilTests(unittest.TestCase):
+
+    def test_get_type(self):
+        self.assertTrue(talloc.Object is _test_pytalloc.get_object_type())
+
+    def test_refrence(self):
+        # Check correct lifetime of the talloc'd data with multiple references
+        lst = []
+        obj = _test_pytalloc.DObject(lambda: lst.append('dead'))
+        ref = _test_pytalloc.reference(obj)
+        del obj
+        gc.collect()
+        self.assertEqual(lst, [])
+        del ref
+        gc.collect()
+        self.assertEqual(lst, ['dead'])
+
+
+if __name__ == '__main__':
+    unittest.TestProgram()
diff --git a/lib/talloc/wscript b/lib/talloc/wscript
index a1b72a0..1367988 100644
--- a/lib/talloc/wscript
+++ b/lib/talloc/wscript
@@ -136,13 +136,30 @@ def build(bld):
                          enabled=True,
                          realname='talloc.so')
 
+        bld.SAMBA_PYTHON('test_pytalloc',
+                         'test_pytalloc.c',
+                         deps='pytalloc',
+                         enabled=True,
+                         realname='_test_pytalloc.so',
+                         install=False)
+
 def test(ctx):
     '''run talloc testsuite'''
     import Utils, samba_utils
+    env = samba_utils.LOAD_ENVIRONMENT()
     cmd = os.path.join(Utils.g_module.blddir, 'talloc_testsuite')
     ret = samba_utils.RUN_COMMAND(cmd)
     print("testsuite returned %d" % ret)
-    sys.exit(ret)
+    if 'USING_SYSTEM_PYTALLOC_UTIL' not in env.defines and not env.disable_python:
+        cmd = "PYTHONPATH=%s %s test_pytalloc.py" % (
+            os.path.join(Utils.g_module.blddir, 'python'),
+            env['PYTHON'],
+        )
+        pyret = samba_utils.RUN_COMMAND(cmd)
+    else:
+        pyret = 0
+    print("python testsuite returned %d" % pyret)
+    sys.exit(ret or pyret)
 
 def dist():
     '''makes a tarball for distribution'''
-- 
2.1.0


From 25bfbf50f6bac78c321e592b5353e78ecdaff572 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori at redhat.com>
Date: Wed, 12 Nov 2014 16:53:33 +0100
Subject: [PATCH 04/15] buildtools: Honor LDVERSION when looking for Python
 library

Since Python 3.2, Python .so files are tagged for ABI compatibility,
so the library name is something like libpython3.4m.so (note the 'm').
This information is found in distutils.sysconfig.get_config_var('LDVERSION')

This fixes waf issue 1405 (https://code.google.com/p/waf/issues/detail?id=1405)

Reviewed-By: Jelmer Vernooij <jelmer at samba.org>
Signed-off-by: Petr Viktorin <pviktori at redhat.com>
---
 third_party/waf/wafadmin/Tools/python.py | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/third_party/waf/wafadmin/Tools/python.py b/third_party/waf/wafadmin/Tools/python.py
index cfc8495..cd96b65 100644
--- a/third_party/waf/wafadmin/Tools/python.py
+++ b/third_party/waf/wafadmin/Tools/python.py
@@ -170,10 +170,10 @@ def check_python_headers(conf, mandatory=True):
 
 	try:
 		# Get some python configuration variables using distutils
-		v = 'prefix SO SYSLIBS LDFLAGS SHLIBS LIBDIR LIBPL INCLUDEPY Py_ENABLE_SHARED MACOSX_DEPLOYMENT_TARGET'.split()
+		v = 'prefix SO SYSLIBS LDFLAGS SHLIBS LIBDIR LIBPL INCLUDEPY Py_ENABLE_SHARED MACOSX_DEPLOYMENT_TARGET LDVERSION'.split()
 		(python_prefix, python_SO, python_SYSLIBS, python_LDFLAGS, python_SHLIBS,
 		 python_LIBDIR, python_LIBPL, INCLUDEPY, Py_ENABLE_SHARED,
-		 python_MACOSX_DEPLOYMENT_TARGET) = \
+		 python_MACOSX_DEPLOYMENT_TARGET, python_LDVERSION) = \
 			_get_python_variables(python, ["get_config_var('%s') or ''" % x for x in v],
 					      ['from distutils.sysconfig import get_config_var'])
 	except RuntimeError:
@@ -190,8 +190,10 @@ python_LIBPL = %r
 INCLUDEPY = %r
 Py_ENABLE_SHARED = %r
 MACOSX_DEPLOYMENT_TARGET = %r
+LDVERSION = %r
 """ % (python, python_prefix, python_SO, python_SYSLIBS, python_LDFLAGS, python_SHLIBS,
-	python_LIBDIR, python_LIBPL, INCLUDEPY, Py_ENABLE_SHARED, python_MACOSX_DEPLOYMENT_TARGET))
+	python_LIBDIR, python_LIBPL, INCLUDEPY, Py_ENABLE_SHARED, python_MACOSX_DEPLOYMENT_TARGET,
+	python_LDVERSION))
 
 	# Allow some python overrides from env vars for cross-compiling
 	os_env = dict(os.environ)
@@ -230,7 +232,9 @@ MACOSX_DEPLOYMENT_TARGET = %r
 		parse_flags(python_LDFLAGS, 'PYEMBED', env)
 
 	result = False
-	name = 'python' + env['PYTHON_VERSION']
+	if not python_LDVERSION:
+		python_LDVERSION = env['PYTHON_VERSION']
+	name = 'python' + python_LDVERSION
 
 	if python_LIBDIR is not None:
 		path = [python_LIBDIR]
@@ -245,7 +249,7 @@ MACOSX_DEPLOYMENT_TARGET = %r
 	if not result:
 		conf.log.write("\n\n# try again with -L$prefix/libs, and pythonXY name rather than pythonX.Y (win32)\n")
 		path = [os.path.join(python_prefix, "libs")]
-		name = 'python' + env['PYTHON_VERSION'].replace('.', '')
+		name = 'python' + python_LDVERSION.replace('.', '')
 		result = conf.check(lib=name, uselib='PYEMBED', libpath=path)
 
 	if result:
-- 
2.1.0


From ee7e633b871183648c66416220bb88144f564569 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori at redhat.com>
Date: Wed, 12 Nov 2014 19:49:45 +0100
Subject: [PATCH 05/15] buildtools: Use all of pyext_PATTERN in
 map_shlib_extension

In Python 3, C extension module filenames have an ABI tag;
the pyext_PATTERN is e.g. "%s.cpython-34m.so".
The build system was only using the last dot-separated element
of that extension (the ".so").

Use the whole extension when constructing the final filename.

Reviewed-By: Jelmer Vernooij <jelmer at samba.org>
Signed-off-by: Petr Viktorin <pviktori at redhat.com>
---
 buildtools/wafsamba/samba_utils.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/buildtools/wafsamba/samba_utils.py b/buildtools/wafsamba/samba_utils.py
index df4a552..e8bc0f3 100644
--- a/buildtools/wafsamba/samba_utils.py
+++ b/buildtools/wafsamba/samba_utils.py
@@ -577,7 +577,7 @@ def map_shlib_extension(ctx, name, python=False):
         return name
     (root1, ext1) = os.path.splitext(name)
     if python:
-        (root2, ext2) = os.path.splitext(ctx.env.pyext_PATTERN)
+        return ctx.env.pyext_PATTERN % root1
     else:
         (root2, ext2) = os.path.splitext(ctx.env.shlib_PATTERN)
     return root1+ext2
-- 
2.1.0


More information about the samba-technical mailing list