Re: [Xen-devel] [PATCH V5 19/32] libxl/gentypes.py: don't generate default values

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-05-13 at 22:54 +0100, Wei Liu wrote:
> If a type has init_val defined and a field of the type has the value of
> init_val, there's no need to generate output for that field.

Please can you explain why there is no need. And I think you need to
mention JSON here somewhere.

> Also define a bunch of init_vals for enumeration types.

I'm not convinced by this. If a type has no initval then you should
compare it to zero, which is a valid thing to do even for an enum.

NB you changed the init_val for some typesto (e.g. from 1 to
LIBXL_VGA_INTERFACE_TYPE_CIRRUS), which is a no semantic change change,
but did mean I had to double check.

Ian.

> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  tools/libxl/gentypes.py     |   27 ++++++++++++++++++++++++---
>  tools/libxl/libxl_types.idl |   27 ++++++++++++++-------------
>  2 files changed, 38 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
> index 01416e7..d2ed12a 100644
> --- a/tools/libxl/gentypes.py
> +++ b/tools/libxl/gentypes.py
> @@ -135,7 +135,7 @@ def _libxl_C_type_init(ty, v, indent = "    ", parent = 
> None, subinit=False):
>              else:
>                  s += _libxl_C_type_init(f.type, fexpr, "", nparent)
>      else:
> -        if ty.init_val is not None:
> +        if ty.init_val is not None and ty.typename != "libxl_defbool":
>              s += "%s = %s;\n" % (ty.pass_arg(v, parent is None), ty.init_val)
>          elif ty.init_fn is not None:
>              s += "%s(%s);\n" % (ty.init_fn, ty.pass_arg(v, parent is None))
> @@ -206,6 +206,13 @@ def libxl_C_type_gen_map_key(f, parent, indent = ""):
>          s = indent + s
>      return s.replace("\n", "\n%s" % indent).rstrip(indent)
>  
> +def get_init_val(f):
> +    if f.init_val is not None:
> +        return f.init_val
> +    elif f.type.init_val is not None:
> +        return f.type.init_val
> +    return None
> +
>  def libxl_C_type_gen_json(ty, v, indent = "    ", parent = None):
>      s = ""
>      if parent is None:
> @@ -255,8 +262,22 @@ def libxl_C_type_gen_json(ty, v, indent = "    ", parent 
> = None):
>          s += "    goto out;\n"
>          for f in [f for f in ty.fields if not f.const and not 
> f.type.private]:
>              (nparent,fexpr) = ty.member(v, f, parent is None)
> -            s += libxl_C_type_gen_map_key(f, nparent)
> -            s += libxl_C_type_gen_json(f.type, fexpr, "", nparent)
> +            init_val = get_init_val(f)
> +            if init_val:
> +                if f.type.typename != "libxl_defbool":
> +                    s += "if (%s != %s) {\n" % (fexpr, init_val)
> +                else:
> +                    s += "if (%s.val != %s) {\n" % (fexpr, init_val)
> +                indent1 = "    "
> +            else:
> +                indent1 = ""
> +
> +            s += libxl_C_type_gen_map_key(f, nparent, indent1)
> +            s += libxl_C_type_gen_json(f.type, fexpr, indent, nparent)
> +
> +            if init_val:
> +                s += "}\n"
> +
>          s += "s = yajl_gen_map_close(hand);\n"
>          s += "if (s != yajl_gen_status_ok)\n"
>          s += "    goto out;\n"
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 2162c37..35da8ba 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -5,7 +5,8 @@
>  
>  namespace("libxl_")
>  
> -libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE)
> +libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE,
> +                        init_val="LIBXL__DEFBOOL_DEFAULT")
>  
>  libxl_domid = Builtin("domid", json_gen_fn = "yajl_gen_integer", 
> autogenerate_json = False)
>  libxl_devid = Builtin("devid", json_gen_fn = "yajl_gen_integer", 
> autogenerate_json = False, signed = True, init_val="-1")
> @@ -55,13 +56,13 @@ libxl_device_model_version = 
> Enumeration("device_model_version", [
>      (0, "UNKNOWN"),
>      (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm)
>      (2, "QEMU_XEN"),             # Upstream based qemu-xen device model
> -    ])
> +    ], init_val = "LIBXL_DEVICE_MODEL_VERSION_UNKNOWN")
>  
>  libxl_console_type = Enumeration("console_type", [
>      (0, "UNKNOWN"),
>      (1, "SERIAL"),
>      (2, "PV"),
> -    ])
> +    ], init_val = "LIBXL_CONSOLE_TYPE_UNKNOWN")
>  
>  libxl_disk_format = Enumeration("disk_format", [
>      (0, "UNKNOWN"),
> @@ -70,20 +71,20 @@ libxl_disk_format = Enumeration("disk_format", [
>      (3, "VHD"),
>      (4, "RAW"),
>      (5, "EMPTY"),
> -    ])
> +    ], init_val = "LIBXL_DISK_FORMAT_UNKNOWN")
>  
>  libxl_disk_backend = Enumeration("disk_backend", [
>      (0, "UNKNOWN"),
>      (1, "PHY"),
>      (2, "TAP"),
>      (3, "QDISK"),
> -    ])
> +    ], init_val = "LIBXL_DISK_BACKEND_UNKNOWN")
>  
>  libxl_nic_type = Enumeration("nic_type", [
>      (0, "UNKNOWN"),
>      (1, "VIF_IOEMU"),
>      (2, "VIF"),
> -    ])
> +    ], init_val = "LIBXL_NIC_TYPE_UNKNOWN")
>  
>  libxl_action_on_shutdown = Enumeration("action_on_shutdown", [
>      (1, "DESTROY"),
> @@ -95,7 +96,7 @@ libxl_action_on_shutdown = 
> Enumeration("action_on_shutdown", [
>  
>      (5, "COREDUMP_DESTROY"),
>      (6, "COREDUMP_RESTART"),
> -    ], init_val = 1)
> +    ], init_val = "LIBXL_ACTION_ON_SHUTDOWN_DESTROY")
>  
>  libxl_trigger = Enumeration("trigger", [
>      (0, "UNKNOWN"),
> @@ -105,14 +106,14 @@ libxl_trigger = Enumeration("trigger", [
>      (4, "INIT"),
>      (5, "RESET"),
>      (6, "S3RESUME"),
> -    ])
> +    ], init_val = "LIBXL_TRIGGER_UNKNOWN")
>  
>  libxl_tsc_mode = Enumeration("tsc_mode", [
>      (0, "default"),
>      (1, "always_emulate"),
>      (2, "native"),
>      (3, "native_paravirt"),
> -    ])
> +    ], init_val = "LIBXL_TSC_MODE_DEFAULT")
>  
>  # Consistent with the values defined for HVM_PARAM_TIMER_MODE.
>  libxl_timer_mode = Enumeration("timer_mode", [
> @@ -128,7 +129,7 @@ libxl_bios_type = Enumeration("bios_type", [
>      (1, "rombios"),
>      (2, "seabios"),
>      (3, "ovmf"),
> -    ])
> +    ], init_val = "LIBXL_BIOS_TYPE_UNKNOWN")
>  
>  # Consistent with values defined in domctl.h
>  # Except unknown which we have made up
> @@ -138,7 +139,7 @@ libxl_scheduler = Enumeration("scheduler", [
>      (5, "credit"),
>      (6, "credit2"),
>      (7, "arinc653"),
> -    ])
> +    ], init_val = "LIBXL_SCHEDULER_UNKNOWN")
>  
>  # Consistent with SHUTDOWN_* in sched.h (apart from UNKNOWN)
>  libxl_shutdown_reason = Enumeration("shutdown_reason", [
> @@ -154,12 +155,12 @@ libxl_vga_interface_type = 
> Enumeration("vga_interface_type", [
>      (1, "CIRRUS"),
>      (2, "STD"),
>      (3, "NONE"),
> -    ], init_val = 1)
> +    ], init_val = "LIBXL_VGA_INTERFACE_TYPE_CIRRUS")
>  
>  libxl_vendor_device = Enumeration("vendor_device", [
>      (0, "NONE"),
>      (1, "XENSERVER"),
> -    ])
> +    ], init_val = "LIBXL_VENDOR_DEVICE_NONE")
>  #
>  # Complex libxl types
>  #



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