| Message ID | 20171002120202.su2p6om5kodlvrj4@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 2, 2017 at 3:02 PM, Wei Liu <wei.liu2@citrix.com> wrote: > On Fri, Sep 29, 2017 at 04:49:23PM +0300, Oleksandr Grytsov wrote: >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com> >> >> Enum always uses "x" value as input argument. In >> case of enum array "t" argument should be passed. >> >> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> > > Checking parent doesn't seem to be necessary. We already have "w" which > is passed by the higher level. > > Can you try the following patch? > > From c451e88dc64febbbea835563eb3347cbc24874ce Mon Sep 17 00:00:00 2001 > From: Wei Liu <wei.liu2@citrix.com> > Date: Mon, 2 Oct 2017 12:48:28 +0100 > Subject: [PATCH] libxl/gentypes: fix generating array of enums > > There is no reason to hardcode "x" in code. Use "w" which is passed > by the higher level. > > This change requires us to allow "x" to be unused so that the > top-level enum parse_json functions continue to compile. > > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > --- > tools/libxl/gentypes.py | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py > index 76aca76aaa..88e5c5f30e 100644 > --- a/tools/libxl/gentypes.py > +++ b/tools/libxl/gentypes.py > @@ -432,7 +432,7 @@ def libxl_C_type_parse_json(ty, w, v, indent = " ", parent = None, discrimina > s = "" > if parent is None: > s += "int rc = 0;\n" > - s += "const libxl__json_object *x = o;\n" > + s += "const libxl__json_object *x __attribute__((__unused__)) = o;\n" > > if isinstance(ty, idl.Array): > if parent is None: > @@ -467,11 +467,11 @@ def libxl_C_type_parse_json(ty, w, v, indent = " ", parent = None, discrimina > raise Exception("Only KeyedUnion can have discriminator") > s += "{\n" > s += " const char *enum_str;\n" > - s += " if (!libxl__json_object_is_string(x)) {\n" > + s += " if (!libxl__json_object_is_string(%s)) {\n" % w > s += " rc = -1;\n" > s += " goto out;\n" > s += " }\n" > - s += " enum_str = libxl__json_object_get_string(x);\n" > + s += " enum_str = libxl__json_object_get_string(%s);\n" % w > s += " rc = %s_from_string(enum_str, %s);\n" % (ty.typename, ty.pass_arg(v, parent is None, idl.PASS_BY_REFERENCE)) > s += " if (rc)\n" > s += " goto out;\n" > -- > 2.11.0 > Checked this patch. It works.
On Mon, Oct 2, 2017 at 4:18 PM, Oleksandr Grytsov <al1img@gmail.com> wrote: > On Mon, Oct 2, 2017 at 3:02 PM, Wei Liu <wei.liu2@citrix.com> wrote: >> On Fri, Sep 29, 2017 at 04:49:23PM +0300, Oleksandr Grytsov wrote: >>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com> >>> >>> Enum always uses "x" value as input argument. In >>> case of enum array "t" argument should be passed. >>> >>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> >> >> Checking parent doesn't seem to be necessary. We already have "w" which >> is passed by the higher level. >> >> Can you try the following patch? >> >> From c451e88dc64febbbea835563eb3347cbc24874ce Mon Sep 17 00:00:00 2001 >> From: Wei Liu <wei.liu2@citrix.com> >> Date: Mon, 2 Oct 2017 12:48:28 +0100 >> Subject: [PATCH] libxl/gentypes: fix generating array of enums >> >> There is no reason to hardcode "x" in code. Use "w" which is passed >> by the higher level. >> >> This change requires us to allow "x" to be unused so that the >> top-level enum parse_json functions continue to compile. >> >> Signed-off-by: Wei Liu <wei.liu2@citrix.com> >> --- >> tools/libxl/gentypes.py | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py >> index 76aca76aaa..88e5c5f30e 100644 >> --- a/tools/libxl/gentypes.py >> +++ b/tools/libxl/gentypes.py >> @@ -432,7 +432,7 @@ def libxl_C_type_parse_json(ty, w, v, indent = " ", parent = None, discrimina >> s = "" >> if parent is None: >> s += "int rc = 0;\n" >> - s += "const libxl__json_object *x = o;\n" >> + s += "const libxl__json_object *x __attribute__((__unused__)) = o;\n" >> >> if isinstance(ty, idl.Array): >> if parent is None: >> @@ -467,11 +467,11 @@ def libxl_C_type_parse_json(ty, w, v, indent = " ", parent = None, discrimina >> raise Exception("Only KeyedUnion can have discriminator") >> s += "{\n" >> s += " const char *enum_str;\n" >> - s += " if (!libxl__json_object_is_string(x)) {\n" >> + s += " if (!libxl__json_object_is_string(%s)) {\n" % w >> s += " rc = -1;\n" >> s += " goto out;\n" >> s += " }\n" >> - s += " enum_str = libxl__json_object_get_string(x);\n" >> + s += " enum_str = libxl__json_object_get_string(%s);\n" % w >> s += " rc = %s_from_string(enum_str, %s);\n" % (ty.typename, ty.pass_arg(v, parent is None, idl.PASS_BY_REFERENCE)) >> s += " if (rc)\n" >> s += " goto out;\n" >> -- >> 2.11.0 >> > > Checked this patch. It works. > > -- > Best Regards, > Oleksandr Grytsov. Hi Wei, will you commit your patch?
On Wed, Oct 04, 2017 at 12:32:05PM +0300, Oleksandr Grytsov wrote: > On Mon, Oct 2, 2017 at 4:18 PM, Oleksandr Grytsov <al1img@gmail.com> wrote: > > On Mon, Oct 2, 2017 at 3:02 PM, Wei Liu <wei.liu2@citrix.com> wrote: > >> On Fri, Sep 29, 2017 at 04:49:23PM +0300, Oleksandr Grytsov wrote: > >>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com> > >>> > >>> Enum always uses "x" value as input argument. In > >>> case of enum array "t" argument should be passed. > >>> > >>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> > >> > >> Checking parent doesn't seem to be necessary. We already have "w" which > >> is passed by the higher level. > >> > >> Can you try the following patch? > >> > >> From c451e88dc64febbbea835563eb3347cbc24874ce Mon Sep 17 00:00:00 2001 > >> From: Wei Liu <wei.liu2@citrix.com> > >> Date: Mon, 2 Oct 2017 12:48:28 +0100 > >> Subject: [PATCH] libxl/gentypes: fix generating array of enums > >> > >> There is no reason to hardcode "x" in code. Use "w" which is passed > >> by the higher level. > >> > >> This change requires us to allow "x" to be unused so that the > >> top-level enum parse_json functions continue to compile. > >> > >> Signed-off-by: Wei Liu <wei.liu2@citrix.com> > >> --- > >> tools/libxl/gentypes.py | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py > >> index 76aca76aaa..88e5c5f30e 100644 > >> --- a/tools/libxl/gentypes.py > >> +++ b/tools/libxl/gentypes.py > >> @@ -432,7 +432,7 @@ def libxl_C_type_parse_json(ty, w, v, indent = " ", parent = None, discrimina > >> s = "" > >> if parent is None: > >> s += "int rc = 0;\n" > >> - s += "const libxl__json_object *x = o;\n" > >> + s += "const libxl__json_object *x __attribute__((__unused__)) = o;\n" > >> > >> if isinstance(ty, idl.Array): > >> if parent is None: > >> @@ -467,11 +467,11 @@ def libxl_C_type_parse_json(ty, w, v, indent = " ", parent = None, discrimina > >> raise Exception("Only KeyedUnion can have discriminator") > >> s += "{\n" > >> s += " const char *enum_str;\n" > >> - s += " if (!libxl__json_object_is_string(x)) {\n" > >> + s += " if (!libxl__json_object_is_string(%s)) {\n" % w > >> s += " rc = -1;\n" > >> s += " goto out;\n" > >> s += " }\n" > >> - s += " enum_str = libxl__json_object_get_string(x);\n" > >> + s += " enum_str = libxl__json_object_get_string(%s);\n" % w > >> s += " rc = %s_from_string(enum_str, %s);\n" % (ty.typename, ty.pass_arg(v, parent is None, idl.PASS_BY_REFERENCE)) > >> s += " if (rc)\n" > >> s += " goto out;\n" > >> -- > >> 2.11.0 > >> > > > > Checked this patch. It works. > > > > -- > > Best Regards, > > Oleksandr Grytsov. > > Hi Wei, will you commit your patch? > Yes, I plan to fix this bug one way or another for this release.
Wei Liu writes ("Re: [PATCH] libxl: fix generating array of enums in getypes.py"):
> From: Wei Liu <wei.liu2@citrix.com>
> Date: Mon, 2 Oct 2017 12:48:28 +0100
> Subject: [PATCH] libxl/gentypes: fix generating array of enums
>
> There is no reason to hardcode "x" in code. Use "w" which is passed
> by the higher level.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
The logic here is rather tangled but I don't fancy refactoring the idl
compiler...
Ian.
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py index 76aca76aaa..88e5c5f30e 100644 --- a/tools/libxl/gentypes.py +++ b/tools/libxl/gentypes.py @@ -432,7 +432,7 @@ def libxl_C_type_parse_json(ty, w, v, indent = " ", parent = None, discrimina s = "" if parent is None: s += "int rc = 0;\n" - s += "const libxl__json_object *x = o;\n" + s += "const libxl__json_object *x __attribute__((__unused__)) = o;\n" if isinstance(ty, idl.Array): if parent is None: @@ -467,11 +467,11 @@ def libxl_C_type_parse_json(ty, w, v, indent = " ", parent = None, discrimina raise Exception("Only KeyedUnion can have discriminator") s += "{\n" s += " const char *enum_str;\n" - s += " if (!libxl__json_object_is_string(x)) {\n" + s += " if (!libxl__json_object_is_string(%s)) {\n" % w s += " rc = -1;\n" s += " goto out;\n" s += " }\n" - s += " enum_str = libxl__json_object_get_string(x);\n" + s += " enum_str = libxl__json_object_get_string(%s);\n" % w s += " rc = %s_from_string(enum_str, %s);\n" % (ty.typename, ty.pass_arg(v, parent is None, idl.PASS_BY_REFERENCE)) s += " if (rc)\n" s += " goto out;\n"