libxl: fix generating array of enums in getypes.py

PATCH New Oleksandr Grytsov 3 participants 7 messages Sep 29, 2017 – Oct 4, 2017

lore

Share & tools

lore

Oleksandr Grytsov

author

2017-09-29 13:49 UTC #0 lore raw

PATCH 1/1 libxl: fix generating array of enums in getypes.py

From: Oleksandr Grytsov <[email protected]>

There is an issue with gentypes.py when using array of enums:

for (i=0; (t=libxl__json_array_get(x,i)); i++) {
    {
        const char *enum_str;
        if (!libxl__json_object_is_string(x)) {
            rc = -1;
            goto out;
        }
        enum_str = libxl__json_object_get_string(x);
        rc = libxl_vsnd_pcm_format_from_string(enum_str, &p->sample_formats[i]);
        if (rc)
            goto out;
    }
}

In above code x is an array and as result condition if (!libxl__json_object_is_string(x)) fails.

The fix is to put "t" instead of "x" when the enum is parsed from array.

Oleksandr Grytsov (1):
libxl: fix generating array of enums in getypes.py

 tools/libxl/gentypes.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Signature · 7 lines

2.7.4


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Oleksandr Grytsov

author

2017-09-29 13:49 UTC #1 lore raw

PATCH 1/1 libxl: fix generating array of enums in getypes.py

From: Oleksandr Grytsov <[email protected]>

Enum always uses "x" value as input argument. In
case of enum array "t" argument should be passed.

Signed-off-by Oleksandr Grytsov <[email protected]>

---
 tools/libxl/gentypes.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

tools/libxl/gentypes.py +5 -2

index 4ea7091..51a5539 100644
@@ -404,13 +404,16 @@ def libxl_C_type_parse_json(ty, w, v, indent = "    ", parent = None, discrimina
     elif isinstance(ty, idl.Enumeration):
         if discriminator is not None:
             raise Exception("Only KeyedUnion can have discriminator")
+        lv = "x"
+        if parent:
+            lv = w;
         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" % lv
         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" % lv
         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"

Signature · 7 lines

2.7.4


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Wei Liu

2017-10-02 12:02 UTC #2 lore raw

Quoted 6 lines · Oleksandr Grytsov

From: Oleksandr Grytsov <[email protected]>
Enum always uses "x" value as input argument. In
case of enum array "t" argument should be passed.
Signed-off-by Oleksandr Grytsov <[email protected]>

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 <[email protected]>
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 <[email protected]>

---
 tools/libxl/gentypes.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

tools/libxl/gentypes.py +3 -3

index 76aca76aaa..88e5c5f30e 100644
@@ -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"

Signature · 7 lines

2.11.0


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Oleksandr Grytsov

author

2017-10-02 13:18 UTC #3 lore raw

Quoted 57 lines · Wei Liu

Quoted 6 lines · Oleksandr Grytsov
From: Oleksandr Grytsov <[email protected]>
Enum always uses "x" value as input argument. In
case of enum array "t" argument should be passed.
Signed-off-by Oleksandr Grytsov <[email protected]>
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 <[email protected]>
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 <[email protected]>
---
 tools/libxl/gentypes.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
tools/libxl/gentypes.py +3 -4
index 76aca76aaa..88e5c5f30e 100644
@@ -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"
--

Quoted 2 lines

2.11.0

Checked this patch. It works.

Signature · 7 lines

Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Oleksandr Grytsov

author

2017-10-04 09:32 UTC #4 lore raw

Quoted 63 lines · Oleksandr Grytsov

Quoted 57 lines · Wei Liu
Quoted 6 lines · Oleksandr Grytsov
From: Oleksandr Grytsov <[email protected]>
Enum always uses "x" value as input argument. In
case of enum array "t" argument should be passed.
Signed-off-by Oleksandr Grytsov <[email protected]>
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 <[email protected]>
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 <[email protected]>
---
 tools/libxl/gentypes.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
tools/libxl/gentypes.py +3 -4
index 76aca76aaa..88e5c5f30e 100644
@@ -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"
--
Quoted 2 lines
2.11.0
Checked this patch. It works.

Quoted 3 lines

--
Best Regards,
Oleksandr Grytsov.

Hi Wei, will you commit your patch?

Signature · 7 lines

Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Wei Liu

2017-10-04 09:43 UTC #5 lore raw

Quoted 64 lines · Oleksandr Grytsov

Quoted 63 lines · Oleksandr Grytsov
Quoted 57 lines · Wei Liu
Quoted 6 lines · Oleksandr Grytsov
From: Oleksandr Grytsov <[email protected]>
Enum always uses "x" value as input argument. In
case of enum array "t" argument should be passed.
Signed-off-by Oleksandr Grytsov <[email protected]>
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 <[email protected]>
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 <[email protected]>
---
 tools/libxl/gentypes.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
tools/libxl/gentypes.py +3 -4
index 76aca76aaa..88e5c5f30e 100644
@@ -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"
--
Quoted 2 lines
2.11.0
Checked this patch. It works.

Quoted 6 lines

Quoted 3 lines
--
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.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Ian Jackson

Acked-by

2017-10-04 10:02 UTC #6 lore raw

In reply to Wei Liu

Wei Liu writes ("Re: [PATCH] libxl: fix generating array of enums in getypes.py"):

Quoted 6 lines

From: Wei Liu <[email protected]>
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 <[email protected]>

The logic here is rather tangled but I don't fancy refactoring the idl compiler...

Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel