[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 07 of 10] pyxl: Recursively scan type-tree to produce complete binding boilerplate



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.