[XEN,for-4.13,v2,4/4] libxl: gentypes: initialise array elements in json

Commit Message

Ian Jackson Oct. 29, 2019, 3:54 p.m. UTC

From: Oleksandr Grytsov <[e-mail]>

Currently, array elements are initialized with calloc.  Which means
initialize all element fields with zero values.  If an entry is not
present in the json (which is entirely permitted), the element will be
all-bits-zero instead of the default value (which is wrong).

The fix is to initalise each array element before parsing it, using
the new libxl_C_type_do_init function.

With existing types this results in a lot of new calls like this:

      for (i=0; (t=libxl__json_array_get(x,i)); i++) {
 +            libxl_sched_params_init(&p->vcpus[i]);
              rc = libxl__sched_params_parse_json(gc, t, &p->vcpus[i]);

(indentation adjusted).  This looks right.  To check what happens with
types which have nontrivial defaults but don't have init functions (of
which we currently have none in arrays), I (Ian) experimentally added:

      ("pnode", uint32), # physical node of this node
      ("vcpus", libxl_bitmap), # vcpus in this node
 +    ("sporks", Array(MemKB, "num_sporks")),
      ])

The result was this:

          for (i=0; (t=libxl__json_array_get(x,i)); i++) {
 +                p->sporks[i] = LIBXL_MEMKB_DEFAULT;
                  rc = libxl__uint64_parse_json(gc, t, &p->sporks[i]);

where the context was added by adding "sporks" and "+" indicates a
line added by this patch, "initialise array elements in json".

Signed-off-by: Oleksandr Grytsov <[e-mail]>
Signed-off-by: Ian Jackson <[e-mail]>
---
v2 [iwj]: Use libxl_C_type_do_init.
          Reword commit message and discuss spork testing.
---
 tools/libxl/gentypes.py | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Anthony PERARD Oct. 30, 2019, 11:31 a.m. UTC | #1

On Tue, Oct 29, 2019 at 03:54:36PM +0000, Ian Jackson wrote:
> diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
> index 124285cd66..c74445f16e 100644
> --- a/tools/libxl/gentypes.py
> +++ b/tools/libxl/gentypes.py
> @@ -461,6 +461,10 @@ def libxl_C_type_parse_json(ty, w, v, indent = "    ", parent = None, discrimina
>          s += "        goto out;\n"
>          s += "    }\n"
>          s += "    for (i=0; (t=libxl__json_array_get(x,i)); i++) {\n"
> +        s += libxl_C_type_do_init(ty.elem_type,
> +                    lambda(by): ("&" if by == idl.PASS_BY_REFERENCE else "")+

The syntax for using `lambda' is without "()" for the list of parameters.
python3 complains about it.

With that fix:
Acked-by: Anthony PERARD <[e-mail]>

> +                                ("%s[i]" % v),
> +                                  need_zero=False, indent=indent+"    ")
>          s += libxl_C_type_parse_json(ty.elem_type, "t", v+"[i]",
>                                       indent + "    ", parent)
>          s += "    }\n"

Thanks,

Ian Jackson Nov. 19, 2019, 4:45 p.m. UTC | #2

Anthony PERARD writes ("Re: [Xen-devel] [XEN PATCH for-4.13 v2 4/4] libxl: gentypes: initialise array elements in json"):
> On Tue, Oct 29, 2019 at 03:54:36PM +0000, Ian Jackson wrote:
> > +                    lambda(by): ("&" if by == idl.PASS_BY_REFERENCE else "")+
> 
> The syntax for using `lambda' is without "()" for the list of parameters.
> python3 complains about it.

Oh.  Thanks.

> With that fix:
> Acked-by: Anthony PERARD <[e-mail]>

Thanks, fixed and pushed.

Ian.

Patch

diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 124285cd66..c74445f16e 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -461,6 +461,10 @@  def libxl_C_type_parse_json(ty, w, v, indent = "    ", parent = None, discrimina
         s += "        goto out;\n"
         s += "    }\n"
         s += "    for (i=0; (t=libxl__json_array_get(x,i)); i++) {\n"
+        s += libxl_C_type_do_init(ty.elem_type,
+                    lambda(by): ("&" if by == idl.PASS_BY_REFERENCE else "")+
+                                ("%s[i]" % v),
+                                  need_zero=False, indent=indent+"    ")
         s += libxl_C_type_parse_json(ty.elem_type, "t", v+"[i]",
                                      indent + "    ", parent)
         s += "    }\n"