Re: [Xen-devel] [PATCH 11/11] libxl: -Wunused-parameter

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-08-01 at 17:24 +0100, Ian Jackson wrote:
> 
>  * The autogenerated function libxl_event_init_type ignores the type
>    parameter. 

Your wish etc...

8<------------------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1343897835 -3600
# Node ID 5bbb555747204f9b1926741416f79ab6b8b02361
# Parent  5feb45a76581091bd267eecccb078afb91db0b8c
libxl: idl: always initialise the KeyedEnum keyvar in the member init function

Previously we only initialised it if an explicit keyvar_init_val was given but
not if the default was implicitly 0.

In the generated code this only changes the unused libxl_event_init_type
function:

 void libxl_event_init_type(libxl_event *p, libxl_event_type type)
 {
+    assert(!p->type);
+    p->type = type;
     switch (p->type) {
     case LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN:
         break;

However I think it is wrong that this function is unused, this and
libxl_event_init should be used by libxl__event_new. As it happens both are
just memset to zero but for correctness we should use the init functions (in
case the IDL changes).

In the generator we also need to properly handle init_var == 0 which the
current if statements incorrectly treat as False. This doesn't actually have
any impact on the generated code.

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

diff -r 5feb45a76581 -r 5bbb55574720 tools/libxl/gentypes.py
--- a/tools/libxl/gentypes.py   Thu Aug 02 09:26:36 2012 +0100
+++ b/tools/libxl/gentypes.py   Thu Aug 02 09:57:15 2012 +0100
@@ -162,17 +162,20 @@ def libxl_C_type_member_init(ty, field):
                                 ku.keyvar.type.make_arg(ku.keyvar.name))
     s += "{\n"
     
-    if ku.keyvar.init_val:
+    if ku.keyvar.init_val is not None:
         init_val = ku.keyvar.init_val
-    elif ku.keyvar.type.init_val:
+    elif ku.keyvar.type.init_val is not None:
         init_val = ku.keyvar.type.init_val
     else:
         init_val = None
         
+    (nparent,fexpr) = ty.member(ty.pass_arg("p"), ku.keyvar, isref=True)
     if init_val is not None:
-        (nparent,fexpr) = ty.member(ty.pass_arg("p"), ku.keyvar, isref=True)
         s += "    assert(%s == %s);\n" % (fexpr, init_val)
-        s += "    %s = %s;\n" % (fexpr, ku.keyvar.name)
+    else:
+        s += "    assert(!%s);\n" % (fexpr)
+    s += "    %s = %s;\n" % (fexpr, ku.keyvar.name)
+
     (nparent,fexpr) = ty.member(ty.pass_arg("p"), field, isref=True)
     s += _libxl_C_type_init(ku, fexpr, parent=nparent, subinit=True)
     s += "}\n"
diff -r 5feb45a76581 -r 5bbb55574720 tools/libxl/libxl_event.c
--- a/tools/libxl/libxl_event.c Thu Aug 02 09:26:36 2012 +0100
+++ b/tools/libxl/libxl_event.c Thu Aug 02 09:57:15 2012 +0100
@@ -1163,7 +1163,10 @@ libxl_event *libxl__event_new(libxl__egc
     libxl_event *ev;
 
     ev = libxl__zalloc(NOGC,sizeof(*ev));
-    ev->type = type;
+
+    libxl_event_init(ev);
+    libxl_event_init_type(ev, type);
+
     ev->domid = domid;
 
     return ev;




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel