On Tue, 2011-01-11 at 19:26 +0000, Ian Jackson wrote:
> Gianni Tedesco writes ("[Xen-devel] [PATCH 07 of 10] pyxl: Recursively scan
> type-tree to produce complete binding boilerplate"):
> > pyxl: Recursively scan type-tree to produce complete binding boilerplate
>
> I applied patches 1-6. They touch only pyxl and didn't break the
> build.
>
> However patch 7 contains this:
>
> > --- a/tools/libxl/libxl.idl Tue Jan 11 16:01:07 2011 +0000
> > +++ b/tools/libxl/libxl.idl Tue Jan 11 16:01:08 2011 +0000
> ...
> > - ("features", string, True),
> > + ("features", string),
>
> This makes a substantive change to both the interface and the
> generated code, and wasn't mentioned in the comment. I'm not
> convinced it's a change that's appropriate during feature freeze.
We discussed this off-list at the time I was writing it. I think with
one of the other guys. We decided that const char * in the interface was
a bad idea since it imposed an allocation policy where no reason for it
actually existed in the code. I could have worked around this by casting
everything in the python bindings but we felt that was an ugly solution
and not a good precedent to set.
You are right that it changes the code and may incorrectly free the ""
allocated in libxl_dm.c - I could strdup() in that instance that but the
interface is changed so we have a problem. Alternatively I can change 2
lines in genwrap.py to emit correct code for this.
> So I'm pushing 1-6 and the rest will have to wait for after the 4.1
> release, I'm afraid.
Your chainsaw.... my baby... :(
Gianni
--
# HG changeset patch
# Parent 760d458df83d738019b404a2dba12959d79ac1dd
pyxl: Recursively scan type-tree to produce complete binding boilerplate
Currently the python wrapper generator ignores non-toplevel types in the IDL.
The KeyedUnion implementation is trivial but for structures embedded in
structures the code becomes more complex. The problem is that when assigning to
a structure (a.b = x) either we:
- copy the c structure from x in to a.b which is unexpected for python
programmers since everything is by reference not by value.
- keep a reference to x which 'shadows' a.b and do the copy just before we
pass the wrapped C structure across the libxl C API boundary.
The latter approach is chosen. The tree of shadow references is maintained in
the auto-generated code and Py_foo_Unshadow() functions are also generated and
exposed for the hand-written part of the xl wrapper to use.
Finally, for anonymous types, a name mangling system is used to translate names
like x.u.hvm.acpi. To support such constructs in python we would need to export
python types for x.u.hvm such as 'xl.anonymous_hvm_0'. Instead of dealing with
this we simply flatten the type tree so that such a field is named
x.u_hvm_apic.
Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
diff -r 760d458df83d tools/python/genwrap.py
--- a/tools/python/genwrap.py Wed Jan 12 12:44:18 2011 +0000
+++ b/tools/python/genwrap.py Wed Jan 12 12:46:57 2011 +0000
@@ -4,7 +4,7 @@ import sys,os
import libxltypes
-(TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING) = range(4)
+(TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING) = xrange(4)
def py_type(ty):
if ty == libxltypes.bool or isinstance(ty, libxltypes.BitField) and
ty.width == 1:
@@ -18,11 +18,16 @@ def py_type(ty):
return TYPE_STRING
return None
+def shadow_fields(ty):
+ return filter(lambda x:x.shadow_type is True, ty.fields)
+
def py_wrapstruct(ty):
l = []
l.append('typedef struct {')
l.append(' PyObject_HEAD;')
l.append(' %s obj;'%ty.typename);
+ for f in shadow_fields(ty):
+ l.append(' Py_%s *%s_ref;'%(f.type.rawname, f.python_name))
l.append('}Py_%s;'%ty.rawname)
l.append('')
return "\n".join(l) + "\n"
@@ -36,33 +41,59 @@ def py_decls(ty):
l = []
l.append('_hidden Py_%s *Py%s_New(void);\n'%(ty.rawname, ty.rawname))
l.append('_hidden int Py%s_Check(PyObject *self);\n'%ty.rawname)
+ if len(shadow_fields(ty)) > 0:
+ l.append('_hidden void Py%s_Unshadow(Py_%s *self);\n'%(ty.rawname,
ty.rawname))
for f in ty.fields:
if py_type(f.type) is not None:
continue
- l.append('_hidden PyObject *attrib__%s_get(%s *%s);'%(\
- fsanitize(f.type.typename), f.type.typename, f.name))
- l.append('_hidden int attrib__%s_set(PyObject *v, %s *%s);'%(\
- fsanitize(f.type.typename), f.type.typename, f.name))
+ l.append('_hidden PyObject *attrib__%s_get(%s *val);'%(\
+ fsanitize(f.type.typename), f.type.typename))
+ l.append('_hidden int attrib__%s_set(PyObject *v, %s *val);'%(\
+ fsanitize(f.type.typename), f.type.typename))
return '\n'.join(l) + "\n"
+def union_check(f, ret, prefix = 'self->obj.'):
+ u_check = []
+ if len(f.condlist):
+ cond = '||'.join(map(lambda (expr,name):('!(' + expr +
')')%('%s%s'%(prefix,name)), f.condlist))
+ u_check.append(' if ( %s ) {'%cond)
+ u_check.append(' PyErr_SetString(PyExc_AttributeError, "Union
key not selected");')
+ u_check.append(' return %s;'%ret)
+ u_check.append(' }')
+ return u_check
+
def py_attrib_get(ty, f):
t = py_type(f.type)
l = []
- l.append('static PyObject *py_%s_%s_get(Py_%s *self, void
*priv)'%(ty.rawname, f.name, ty.rawname))
+ l.append('static PyObject *py_%s_%s_get(Py_%s *self, void
*priv)'%(ty.rawname, f.python_name, ty.rawname))
l.append('{')
+
+ u_check = union_check(f, 'NULL')
+
if t == TYPE_BOOL:
l.append(' PyObject *ret;')
+ l.extend(u_check)
l.append(' ret = (self->obj.%s) ? Py_True : Py_False;'%f.name)
l.append(' Py_INCREF(ret);')
l.append(' return ret;')
elif t == TYPE_INT:
+ l.extend(u_check)
l.append(' return genwrap__ll_get(self->obj.%s);'%f.name)
elif t == TYPE_UINT:
+ l.extend(u_check)
l.append(' return genwrap__ull_get(self->obj.%s);'%f.name)
elif t == TYPE_STRING:
- l.append(' return genwrap__string_get(&self->obj.%s);'%f.name)
+ l.extend(u_check)
+ l.append(' return genwrap__string_get((char
**)&self->obj.%s);'%f.name)
+ elif f.shadow_type is True:
+ l.append(' PyObject *ret;')
+ l.extend(u_check)
+ l.append(' ret = (self->%s_ref == NULL) ? Py_None : (PyObject
*)self->%s_ref;'%(f.python_name, f.python_name))
+ l.append(' Py_INCREF(ret);')
+ l.append(' return ret;')
else:
tn = f.type.typename
+ l.extend(u_check)
l.append(' return attrib__%s_get((%s
*)&self->obj.%s);'%(fsanitize(tn), tn, f.name))
l.append('}')
return '\n'.join(l) + "\n\n"
@@ -70,14 +101,17 @@ def py_attrib_get(ty, f):
def py_attrib_set(ty, f):
t = py_type(f.type)
l = []
- l.append('static int py_%s_%s_set(Py_%s *self, PyObject *v, void
*priv)'%(ty.rawname, f.name, ty.rawname))
+ l.append('static int py_%s_%s_set(Py_%s *self, PyObject *v, void
*priv)'%(ty.rawname, f.python_name, ty.rawname))
l.append('{')
+ u_check = union_check(f, '-1')
if t == TYPE_BOOL:
+ l.extend(u_check)
l.append(' self->obj.%s = (NULL == v || Py_None == v || Py_False ==
v) ? 0 : 1;'%f.name)
l.append(' return 0;')
elif t == TYPE_UINT or t == TYPE_INT:
l.append(' %slong long tmp;'%(t == TYPE_UINT and 'unsigned ' or ''))
l.append(' int ret;')
+ l.extend(u_check)
if t == TYPE_UINT:
l.append(' ret = genwrap__ull_set(v, &tmp,
(%s)~0);'%f.type.typename)
else:
@@ -86,47 +120,87 @@ def py_attrib_set(ty, f):
l.append(' self->obj.%s = tmp;'%f.name)
l.append(' return ret;')
elif t == TYPE_STRING:
- l.append(' return genwrap__string_set(v, &self->obj.%s);'%f.name)
+ l.extend(u_check)
+ l.append(' return genwrap__string_set(v, (char
**)&self->obj.%s);'%f.name)
+ elif f.shadow_type is True:
+ l.extend(u_check)
+ l.append(' if ( !Py%s_Check(v) ) {'%f.type.rawname)
+ l.append(' PyErr_SetString(PyExc_TypeError, "Expected
xl.%s");'%f.type.rawname)
+ l.append(' return -1;')
+ l.append(' }')
+ l.append(' if ( self->%s_ref ) {'%f.python_name)
+ l.append(' Py_DECREF(self->%s_ref);'%f.python_name)
+ l.append(' }')
+ l.append(' self->%s_ref = (Py_%s *)v;'%(f.python_name,
f.type.rawname))
+ l.append(' Py_INCREF(self->%s_ref);'%f.python_name)
+ l.append(' return 0;')
else:
tn = f.type.typename
+ l.extend(u_check)
l.append(' return attrib__%s_set(v, (%s
*)&self->obj.%s);'%(fsanitize(tn), tn, f.name))
l.append('}')
return '\n'.join(l) + "\n\n"
+def sync_func(ty):
+ pf = shadow_fields(ty)
+ if len(pf) == 0:
+ return ''
+ l = []
+
+ l.append('void Py%s_Unshadow(Py_%s*self)\n'%(ty.rawname, ty.rawname))
+ l.append('{')
+ for f in pf:
+ l.append(' if ( self->%s_ref ) {'%f.python_name)
+ l.append(' Py_%s *x = (Py_%s *)self->%s_ref;'%(f.type.rawname,
f.type.rawname, f.python_name))
+ if len(shadow_fields(f.type)):
+ l.append(' Py%s_Unshadow(x);'%f.type.rawname)
+ l.append(' memcpy(&self->obj.%s, &x->obj,
sizeof(self->obj.%s));'%(f.name, f.name))
+ l.append(' }else{')
+ l.append(' memset(&self->obj.%s, 0,
sizeof(self->obj.%s));'%(f.name, f.name))
+ l.append(' }')
+ l.append('}')
+ l.append('')
+ return '\n'.join(l)
+
def py_object_def(ty):
l = []
- if ty.destructor_fn is not None:
- dtor = ' %s(&self->obj);\n'%ty.destructor_fn
- else:
- dtor = ''
-
- funcs="""static void Py%(rawname)s_dealloc(Py_%(rawname)s *self)
-{
-%(dtor)s self->ob_type->tp_free((PyObject *)self);
-}
-
-static int Py%(rawname)s_init(Py_%(rawname)s *self, PyObject *args, PyObject
*kwds)
+ funcs="""static int Py%s_init(Py_%s *self, PyObject *args, PyObject *kwds)
{
memset(&self->obj, 0, sizeof(self->obj));
return genwrap__obj_init((PyObject *)self, args, kwds);
}
-static PyObject *Py%(rawname)s_new(PyTypeObject *type, PyObject *args,
PyObject *kwds)
+static PyObject *Py%s_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
{
- Py_%(rawname)s *self = (Py_%(rawname)s *)type->tp_alloc(type, 0);
+ Py_%s *self = (Py_%s *)type->tp_alloc(type, 0);
if (self == NULL)
return NULL;
memset(&self->obj, 0, sizeof(self->obj));
return (PyObject *)self;
}
-"""%{'rawname': ty.rawname, 'dtor': dtor}
+"""%tuple(ty.rawname for x in range(5))
+
+ funcs += sync_func(ty)
+
+
+ l.append('static void Py%s_dealloc(Py_%s *self)'%(ty.rawname, ty.rawname))
+ l.append('{')
+ if ty.destructor_fn is not None:
+ l.append(' %s(&self->obj);'%ty.destructor_fn)
+ for f in shadow_fields(ty):
+ l.append(' if ( self->%s_ref ) {'%f.python_name)
+ l.append(' Py_DECREF(self->%s_ref);'%f.python_name)
+ l.append(' }')
+ l.append('self->ob_type->tp_free((PyObject *)self);')
+ l.append('}')
+ l.append('')
l.append('static PyGetSetDef Py%s_getset[] = {'%ty.rawname)
for f in ty.fields:
- l.append(' { .name = "%s", '%f.name)
- l.append(' .get = (getter)py_%s_%s_get, '%(ty.rawname, f.name))
- l.append(' .set = (setter)py_%s_%s_set },'%(ty.rawname, f.name))
+ l.append(' { .name = "%s", '%f.python_name)
+ l.append(' .get = (getter)py_%s_%s_get, '%(ty.rawname,
f.python_name))
+ l.append(' .set = (setter)py_%s_%s_set },'%(ty.rawname,
f.python_name))
l.append(' { .name = NULL }')
l.append('};')
struct="""
@@ -196,12 +270,62 @@ def py_initfuncs(types):
l.append('}')
return '\n'.join(l) + "\n\n"
-def tree_frob(types):
- ret = types[:]
- for ty in ret:
- ty.fields = filter(lambda f:f.name is not None and f.type.typename is
not None, ty.fields)
+def dbg_tree(str, indent=0):
+ do_dbg = True
+ if not do_dbg:
+ return
+ print '%s%s'%(''.join(' ' for i in xrange(indent)), str)
+
+# We don't have a good translation for anonymous structures so we just
+# flatten them out recursively and replace '.' with '_'. For example
+# domain_build_info.u.hvm.pae becomes domain_build_info.u_hvm_pae
+def flatten_type(ty, path = None, depth = 0, condvar = []):
+ if not isinstance(ty, libxltypes.Aggregate):
+ return ty.fields
+
+ if path is None:
+ path = ''
+ else:
+ path = path + '.'
+
+ ret = []
+ for f in ty.fields:
+ f.name = path + f.name
+ f.python_name = f.name.replace('.', '_')
+ if isinstance(f.type, libxltypes.Aggregate) and \
+ f.type.typename is not None:
+ f.shadow_type = True
+ else:
+ f.shadow_type = False
+
+ if isinstance(f.type, libxltypes.KeyedUnion):
+ f.type.keyvar_name = path + f.type.keyvar_name
+
+ f.condlist = condvar[:]
+
+ if isinstance(f.type, libxltypes.Aggregate) and \
+ f.type.typename is None:
+ dbg_tree('(%s)'%(f.name), depth + 1)
+ if isinstance(ty, libxltypes.KeyedUnion):
+ condvar.append((f.keyvar_expr, ty.keyvar_name))
+ ret.extend(flatten_type(f.type, f.name, depth + 1, condvar))
+ if isinstance(ty, libxltypes.KeyedUnion):
+ condvar.pop()
+ else:
+ dbg_tree('%s - %s'%(f.name, f.python_name), depth + 1)
+ ret.append(f)
+
+
return ret
+def frob_type(type):
+ dbg_tree('[%s]'%type.typename)
+ type.fields = flatten_type(type)
+ return type
+
+def frob_types(types):
+ return map(frob_type, types)
+
if __name__ == '__main__':
if len(sys.argv) < 4:
print >>sys.stderr, "Usage: genwrap.py <idl> <decls> <defns>"
@@ -210,7 +334,7 @@ if __name__ == '__main__':
idl = sys.argv[1]
(_,types) = libxltypes.parse(idl)
- types = tree_frob(types)
+ types = frob_types(types)
decls = sys.argv[2]
f = open(decls, 'w')
@@ -250,7 +374,7 @@ _hidden int genwrap__ll_set(PyObject *v,
""" % " ".join(sys.argv))
for ty in types:
- f.write('/* Internal APU for %s wrapper */\n'%ty.typename)
+ f.write('/* Internal API for %s wrapper */\n'%ty.typename)
f.write(py_wrapstruct(ty))
f.write(py_decls(ty))
f.write('\n')
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home