[PATCH 8 of 9] libxl: generate destructors for each libxl defined type

[Ian Campbell <ian.campbell@citrix.com>]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1281707272 -3600
# Node ID 9b62c9e42f2472aaf4a3d9ce492c5c14cdfa8249
# Parent  eb6b31dba5994e748d1aaf3f3eb60ba46e3ae7c3
libxl: generate destructors for each libxl defined type

I chose the name "_destroy" rather than "_free" because the destructor
functions will free only the members of a type recursively but will
not free the actual type structure itself. The allocation of the type
is typically done by the caller and may not be a single allocation,
e.g. lists/arrays of types or embedded in other strucutures etc.

The exceptions to this rule are libxl_string_list_destroy and
libxl_key_value_list_destroy but I'm not 100% convinced they are
exceptions (since they are kind-of opaque) and I couldn't see a
cleanerway to express this concept. I have made a best effort attempt
to implement these functions sanely but since as far as I can tell
nothing in the current code base ever sets
libxl_domain_create_info.{xsdata,platformdata} I'm flying somewhat
blind.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r eb6b31dba599 -r 9b62c9e42f24 .hgignore
--- a/.hgignore	Fri Aug 13 14:46:36 2010 +0100
+++ b/.hgignore	Fri Aug 13 14:47:52 2010 +0100
@@ -182,6 +182,7 @@
 ^tools/libxen/test/test_bindings$
 ^tools/libxen/test/test_event_handling$
 ^tools/libxl/_.*\.h$
+^tools/libxl/_.*\.c$
 ^tools/libxl/libxlu_cfg_y\.output$
 ^tools/libxl/xl$
 ^tools/libaio/src/.*\.ol$
diff -r eb6b31dba599 -r 9b62c9e42f24 tools/libxl/Makefile
--- a/tools/libxl/Makefile	Fri Aug 13 14:46:36 2010 +0100
+++ b/tools/libxl/Makefile	Fri Aug 13 14:47:52 2010 +0100
@@ -19,6 +19,7 @@ LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_lib
 
 LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o
 LIBXL_OBJS = flexarray.o libxl.o libxl_pci.o libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o libxl_internal.o xenguest.o libxl_utils.o $(LIBXL_OBJS-y)
+LIBXL_OBJS += _libxl_types.o
 
 AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h
 AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c
@@ -58,9 +59,10 @@ libxl.h: _libxl_types.h
 
 $(LIBXL_OBJS:%.o=%.c) $(LIBXLU_OBJS:%.o=%.c) $(XL_OBJS:%.o=%.c): libxl.h
 
-_libxl_types.h: libxltypes.idl gentypes.py libxltypes.py
-	python gentypes.py libxltypes.idl  __libxl_types.h
+_libxl_types.h _libxl_types.c: gentypes.py libxltypes.py
+	python gentypes.py libxltypes.idl __libxl_types.h __libxl_types.c
 	mv __libxl_types.h _libxl_types.h
+	mv __libxl_types.c _libxl_types.c
 
 libxenlight.so: libxenlight.so.$(MAJOR)
 	ln -sf $< $@
@@ -110,6 +112,7 @@ install: all
 .PHONY: clean
 clean:
 	$(RM) -f _*.h *.o *.so* *.a $(CLIENTS) $(DEPS)
+	$(RM) -f _*.c
 #	$(RM) -f $(AUTOSRCS) $(AUTOINCS)
 
 distclean: clean
diff -r eb6b31dba599 -r 9b62c9e42f24 tools/libxl/gentypes.py
--- a/tools/libxl/gentypes.py	Fri Aug 13 14:46:36 2010 +0100
+++ b/tools/libxl/gentypes.py	Fri Aug 13 14:47:52 2010 +0100
@@ -59,16 +59,53 @@ def libxl_C_type_define(ty, indent = "")
         raise NotImplementedError("%s" % type(ty))
     return s.replace("\n", "\n%s" % indent)
 
+def libxl_C_type_destroy(ty, v, reference, indent = "    ", parent = None):
+    if reference:
+        deref = v + "->"
+    else:
+        deref = v + "."
+
+    s = ""
+    if isinstance(ty, libxltypes.KeyedUnion):
+        if parent is None:
+            raise Exception("KeyedUnion type must have a parent")
+        for f in ty.fields:
+            keyvar_expr = f.keyvar_expr % (parent + ty.keyvar_name)
+            s += "if (" + keyvar_expr + ") {\n"
+            s += libxl_C_type_destroy(f.type, deref + f.name, False, indent + "    ", deref)
+            s += "}\n"
+    elif isinstance(ty, libxltypes.Reference):
+        s += libxl_C_type_destroy(ty.ref_type, v, True, indent, v)
+    elif isinstance(ty, libxltypes.Struct) and (parent is None or ty.destructor_fn is None):
+        for f in [f for f in ty.fields if not f.const]:
+
+            if f.name is None: # Anonynous struct
+                s += libxl_C_type_destroy(f.type, deref, False, "", deref)
+            else:
+                s += libxl_C_type_destroy(f.type, deref + f.name, False, "", deref)
+    else:
+        if ty.passby == libxltypes.PASS_BY_REFERENCE and not reference:
+            makeref = "&"
+        else:
+            makeref = ""
+
+        if ty.destructor_fn is not None:
+            s += "%s(%s);\n" % (ty.destructor_fn, makeref + v)
+            
+    if s != "":
+        s = indent + s
+    return s.replace("\n", "\n%s" % indent).rstrip(indent)
+
 if __name__ == '__main__':
-    if len(sys.argv) < 3:
-        print >>sys.stderr, "Usage: gentypes.py <idl> <header>"
+    if len(sys.argv) < 4:
+        print >>sys.stderr, "Usage: gentypes.py <idl> <header> <implementation>"
         sys.exit(1)
 
     idl = sys.argv[1]
     (_,types) = libxltypes.parse(idl)
                     
     header = sys.argv[2]
-    print "outputting libxl types to %s" % header
+    print "outputting libxl type definitions to %s" % header
 
     f = open(header, "w")
     
@@ -84,8 +121,39 @@ if __name__ == '__main__':
  
 """ % " ".join(sys.argv))
         
-    for t in types:
-        f.write(libxl_C_type_define(t) + ";\n")
+    for ty in types:
+        f.write(libxl_C_type_define(ty) + ";\n")
+        if ty.destructor_fn is not None:
+            f.write("void %s(%s *p);\n" % (ty.destructor_fn, ty.typename))
         f.write("\n")
 
     f.write("""#endif /* __LIBXL_TYPES_H */\n""")
+    f.close()
+    
+    impl = sys.argv[3]
+    print "outputting libxl type implementations to %s" % impl
+
+    f = open(impl, "w")
+    f.write("""
+/* DO NOT EDIT.
+ *
+ * This file is autogenerated by
+ * "%s"
+ */
+
+#include "libxl_osdeps.h"
+
+#include <stdint.h>
+#include <stdlib.h>
+ 
+#include "libxl.h"
+
+""" % " ".join(sys.argv))
+
+    for ty in [t for t in types if t.autogenerate_destructor]:
+        f.write("void %s(%s *p)\n" % (ty.destructor_fn, ty.typename))
+        f.write("{\n")
+        f.write(libxl_C_type_destroy(ty, "p", True))
+        f.write("}\n")
+        f.write("\n")
+    f.close()
diff -r eb6b31dba599 -r 9b62c9e42f24 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Fri Aug 13 14:46:36 2010 +0100
+++ b/tools/libxl/libxl.c	Fri Aug 13 14:47:52 2010 +0100
@@ -72,6 +72,33 @@ int libxl_ctx_free(libxl_ctx *ctx)
     do_free_version_info(&ctx->version_info);
     if (ctx->xsh) xs_daemon_close(ctx->xsh); 
     return 0;
+}
+
+void libxl_string_list_destroy(libxl_string_list sl)
+{
+    int i;
+
+    if (!sl)
+        return;
+
+    for (i = 0; sl[i] != NULL; i++)
+        free(sl[i]);
+    free(sl);
+}
+
+void libxl_key_value_list_destroy(libxl_key_value_list kvl)
+{
+    int i;
+
+    if (!kvl)
+        return;
+
+    for (i = 0; kvl[i] != NULL; i += 2) {
+        free(kvl[i]);
+        if (kvl[i + 1])
+            free(kvl[i + 1]);
+    }
+    free(kvl);
 }
 
 /******************************************************************************/
diff -r eb6b31dba599 -r 9b62c9e42f24 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Fri Aug 13 14:46:36 2010 +0100
+++ b/tools/libxl/libxl.h	Fri Aug 13 14:47:52 2010 +0100
@@ -27,8 +27,10 @@ typedef uint8_t libxl_mac[6];
 typedef uint8_t libxl_mac[6];
 
 typedef char **libxl_string_list;
+void libxl_string_list_destroy(libxl_string_list sl);
 
 typedef char **libxl_key_value_list;
+void libxl_key_value_list_destroy(libxl_key_value_list kvl);
 
 typedef enum {
     XENFV = 1,
diff -r eb6b31dba599 -r 9b62c9e42f24 tools/libxl/libxltypes.idl
--- a/tools/libxl/libxltypes.idl	Fri Aug 13 14:46:36 2010 +0100
+++ b/tools/libxl/libxltypes.idl	Fri Aug 13 14:47:52 2010 +0100
@@ -11,8 +11,8 @@ libxl_disk_phystype = Builtin("disk_phys
 libxl_disk_phystype = Builtin("disk_phystype")
 libxl_nic_type = Builtin("nic_type")
 
-libxl_string_list = Builtin("string_list")
-libxl_key_value_list = Builtin("key_value_list")
+libxl_string_list = Builtin("string_list", destructor_fn="libxl_string_list_destroy")
+libxl_key_value_list = Builtin("key_value_list", destructor_fn="libxl_key_value_list_destroy")
 
 #
 # Complex libxl types
diff -r eb6b31dba599 -r 9b62c9e42f24 tools/libxl/libxltypes.py
--- a/tools/libxl/libxltypes.py	Fri Aug 13 14:46:36 2010 +0100
+++ b/tools/libxl/libxltypes.py	Fri Aug 13 14:47:52 2010 +0100
@@ -1,9 +1,16 @@ import sys
 import sys
+
+PASS_BY_VALUE = 1
+PASS_BY_REFERENCE = 2
 
 class Type(object):
     def __init__(self, typename, **kwargs):
         self.comment = kwargs.setdefault('comment', None)
         self.namespace = kwargs.setdefault('namespace', "libxl_")
+
+        self.passby = kwargs.setdefault('passby', PASS_BY_VALUE)
+        if self.passby not in [PASS_BY_VALUE, PASS_BY_REFERENCE]:
+            raise ValueError
 
         if typename is None: # Anonymous type
             self.typename = None
@@ -12,14 +19,23 @@ class Type(object):
         else:
             self.typename = self.namespace + typename
 
+        if self.typename is not None:
+            self.destructor_fn = kwargs.setdefault('destructor_fn', self.typename + "_destroy")
+        else:
+            self.destructor_fn = kwargs.setdefault('destructor_fn', None)
+
+        self.autogenerate_destructor = kwargs.setdefault('autogenerate_destructor', True)
+
 class Builtin(Type):
     """Builtin type"""
     def __init__(self, typename, **kwargs):
+        kwargs.setdefault('destructor_fn', None)
         Type.__init__(self, typename, **kwargs)
 
 class UInt(Type):
     def __init__(self, w, **kwargs):
         kwargs.setdefault('namespace', None)
+        kwargs.setdefault('destructor_fn', None)
         Type.__init__(self, "uint%d_t" % w, **kwargs)
 
         self.width = w
@@ -27,6 +43,7 @@ class BitField(Type):
 class BitField(Type):
     def __init__(self, ty, w, **kwargs):
         kwargs.setdefault('namespace', None)
+        kwargs.setdefault('destructor_fn', None)
         Type.__init__(self, ty.typename, **kwargs)
 
         self.width = w
@@ -63,10 +80,16 @@ class Aggregate(Type):
 
 class Struct(Aggregate):
     def __init__(self, name, fields, **kwargs):
+        kwargs.setdefault('passby', PASS_BY_REFERENCE)
         Aggregate.__init__(self, "struct", name, fields, **kwargs)
 
 class Union(Aggregate):
     def __init__(self, name, fields, **kwargs):
+        # Generally speaking some intelligence is required to free a
+        # union therefore any specific instance of this class will
+        # need to provide an explicit destructor function.
+        kwargs.setdefault('passby', PASS_BY_REFERENCE)
+        kwargs.setdefault('destructor_fn', None)
         Aggregate.__init__(self, "union", name, fields, **kwargs)
 
 class KeyedUnion(Aggregate):
@@ -87,7 +110,14 @@ class Reference(Type):
 class Reference(Type):
     """A reference to another type"""
     def __init__(self, ty, **kwargs):
+        self.ref_type = ty
+        
         # Ugh
+        
+        kwargs.setdefault('destructor_fn', "free")
+        kwargs.setdefault('autogenerate_destructor', False)
+        kwargs.setdefault('passby', PASS_BY_VALUE)
+        
         kwargs.setdefault('namespace', ty.namespace)
         typename = ty.typename[len(kwargs['namespace']):]
         Type.__init__(self, typename + " *", **kwargs)
@@ -112,7 +142,7 @@ uint64 = UInt(64)
 
 domid = UInt(32)
 
-string = Builtin("char *", namespace = None)
+string = Builtin("char *", namespace = None, destructor_fn = "free")
 
 inaddr_ip = Builtin("struct in_addr", namespace = None)