qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] [PATCH v6 23/23] qapi: Change visit_type_FOO() to no longer


From: Eric Blake
Subject: [Qemu-devel] [PATCH v6 23/23] qapi: Change visit_type_FOO() to no longer return partial objects
Date: Wed, 25 Nov 2015 17:23:20 -0700

Returning a partial object on error is an invitation for a careless
caller to leak memory.  As no one outside the testsuite was actually
relying on these semantics, it is cleaner to just document and
guarantee that ALL visit_type_FOO() functions leave a safe value
in *obj when an error is encountered during an input visitor.

Since input visitors have blind assignment semantics, we have to
track the result of whether an assignment is made all the way down
to each visitor callback implementation.

Signed-off-by: Eric Blake <address@hidden>

---
v6: rebase on top of earlier doc and formatting improvements, mention
that *obj can be uninitialized on entry to an input visitor, rework
semantics to keep valgrind happy on uninitialized input, break some
long lines
---
 include/qapi/visitor-impl.h    |  6 ++---
 include/qapi/visitor.h         | 52 +++++++++++++++++++++++++++++++-----------
 qapi/opts-visitor.c            |  8 ++++---
 qapi/qapi-dealloc-visitor.c    |  9 +++++---
 qapi/qapi-visit-core.c         | 13 ++++++-----
 qapi/qmp-input-visitor.c       | 17 +++++++++-----
 qapi/qmp-output-visitor.c      |  6 +++--
 qapi/string-input-visitor.c    |  3 ++-
 qapi/string-output-visitor.c   |  3 ++-
 scripts/qapi-visit.py          | 40 ++++++++++++++++++++++++--------
 tests/test-qmp-commands.c      | 13 +++++------
 tests/test-qmp-input-strict.c  | 19 +++++++--------
 tests/test-qmp-input-visitor.c | 10 ++------
 13 files changed, 125 insertions(+), 74 deletions(-)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 5350bdf..a51aa2a 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -26,7 +26,7 @@ struct Visitor
 {
     /* Must be provided to visit structs (the string visitors do not
      * currently visit structs). */
-    void (*start_struct)(Visitor *v, void **obj, const char *kind,
+    bool (*start_struct)(Visitor *v, void **obj, const char *kind,
                          const char *name, size_t size, Error **errp);
     /* May be NULL; most useful for input visitors. */
     void (*check_struct)(Visitor *v, Error **errp);
@@ -34,13 +34,13 @@ struct Visitor
     void (*end_struct)(Visitor *v);

     /* May be NULL; most useful for input visitors. */
-    void (*start_implicit_struct)(Visitor *v, void **obj, size_t size,
+    bool (*start_implicit_struct)(Visitor *v, void **obj, size_t size,
                                   Error **errp);
     /* May be NULL */
     void (*end_implicit_struct)(Visitor *v);

     /* Must be set */
-    void (*start_list)(Visitor *v, const char *name, Error **errp);
+    bool (*start_list)(Visitor *v, const char *name, Error **errp);
     /* Must be set */
     GenericList *(*next_list)(Visitor *v, GenericList **list, Error **errp);
     /* Must be set */
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index f9ea325..0b63a7a 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -30,6 +30,27 @@
  * visitor-impl.h.
  */

+/* All qapi types have a corresponding function with a signature
+ * roughly compatible with this:
+ *
+ * void visit_type_FOO(Visitor *v, void *obj, const char *name, Error **errp);
+ *
+ * where address@hidden is itself a pointer or a scalar.  The visit functions 
for
+ * built-in types are declared here, while the functions for qapi-defined
+ * struct, union, enum, and list types are generated (see qapi-visit.h).
+ * Input visitors may receive an uninitialized address@hidden, and guarantee a
+ * safe value is assigned (a new object on success, or NULL on failure).
+ * Output visitors expect address@hidden to be properly initialized on entry.
+ *
+ * Additionally, all qapi structs have a generated function compatible
+ * with this:
+ *
+ * void qapi_free_FOO(void *obj);
+ *
+ * which behaves like free(), even if @obj is NULL or was only partially
+ * allocated before encountering an error.
+ */
+
 /* This struct is layout-compatible with all other *List structs
  * created by the qapi generator. */
 typedef struct GenericList
@@ -51,10 +72,12 @@ typedef struct GenericList
  * bytes, without regards to the previous value of address@hidden For other
  * visitors, address@hidden is the object to visit. Set address@hidden on 
failure.
  *
- * FIXME: address@hidden can be modified even on error; this can lead to
- * memory leaks if clients aren't careful.
+ * Returns true if address@hidden was allocated; if that happens, and an error
+ * occurs any time before the matching visit_end_struct(), then the
+ * caller (usually a visit_type_FOO() function) knows to undo the
+ * allocation before returning control further.
  */
-void visit_start_struct(Visitor *v, void **obj, const char *kind,
+bool visit_start_struct(Visitor *v, void **obj, const char *kind,
                         const char *name, size_t size, Error **errp);
 /**
  * Prepare for completing a struct visit.
@@ -73,14 +96,12 @@ void visit_end_struct(Visitor *v);

 /**
  * Prepare to visit an implicit struct.
- * Similar to visit_start_struct(), except that in implicit struct
- * represents a subset of keys that are present at the same nesting level
- * of a common object, rather than a new object at a deeper nesting level.
- *
- * FIXME: address@hidden can be modified even on error; this can lead to
- * memory leaks if clients aren't careful.
+ * Similar to visit_start_struct(), including return semantics, except
+ * that an implicit struct represents a subset of keys that are present
+ * at the same nesting level of a common object, rather than a new object
+ * at a deeper nesting level.
  */
-void visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
+bool visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
                                  Error **errp);
 /**
  * Complete an implicit struct visit started earlier.
@@ -96,9 +117,12 @@ void visit_end_implicit_struct(Visitor *v);
  * @name will be NULL if this is visited as part of another list.
  * After calling this, the elements must be collected until
  * visit_next_list() returns NULL, then visit_end_list() must be
- * used to complete the visit.
+ * used to complete the visit.  Return true if the visit may
+ * result in allocation, such that the caller (usually a visit_type_FOO()
+ * function) knows to undo any allocation if an error occurs before
+ * the matching visit_end_list().
  */
-void visit_start_list(Visitor *v, const char *name, Error **errp);
+bool visit_start_list(Visitor *v, const char *name, Error **errp);
 /**
  * Collect the next list member and append it to address@hidden
  * Start with address@hidden of NULL, then subsequent iterations should pass
@@ -106,7 +130,9 @@ void visit_start_list(Visitor *v, const char *name, Error 
**errp);
  * loop until a NULL return or error occurs; for each non-NULL return,
  * the caller must then call the appropriate visit_type_*() for the
  * element type of the list, with that function's name parameter set
- * to NULL.
+ * to NULL.  If an error occurs, then the caller (usually a
+ * visit_type_FOO() function) knows to undo the list allocation before
+ * returning control further.
  */
 GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp);
 /**
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index db5fc1a..33c1868 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -117,7 +117,7 @@ opts_visitor_insert(GHashTable *unprocessed_opts, const 
QemuOpt *opt)
 }


-static void
+static bool
 opts_start_struct(Visitor *v, void **obj, const char *kind,
                   const char *name, size_t size, Error **errp)
 {
@@ -128,7 +128,7 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
         *obj = g_malloc0(size > 0 ? size : 1);
     }
     if (ov->depth++ > 0) {
-        return;
+        return true;
     }

     ov->unprocessed_opts = g_hash_table_new_full(&g_str_hash, &g_str_equal,
@@ -147,6 +147,7 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
         ov->fake_id_opt->str = g_strdup(ov->opts_root->id);
         opts_visitor_insert(ov->unprocessed_opts, ov->fake_id_opt);
     }
+    return true;
 }


@@ -205,7 +206,7 @@ lookup_distinct(const OptsVisitor *ov, const char *name, 
Error **errp)
 }


-static void
+static bool
 opts_start_list(Visitor *v, const char *name, Error **errp)
 {
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
@@ -216,6 +217,7 @@ opts_start_list(Visitor *v, const char *name, Error **errp)
     if (ov->repeated_opts != NULL) {
         ov->list_mode = LM_STARTED;
     }
+    return true;
 }


diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 14dc7c3..a7dc43f 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -59,12 +59,13 @@ static void *qapi_dealloc_pop(QapiDeallocVisitor *qov)
     return value;
 }

-static void qapi_dealloc_start_struct(Visitor *v, void **obj, const char *kind,
+static bool qapi_dealloc_start_struct(Visitor *v, void **obj, const char *kind,
                                       const char *name, size_t unused,
                                       Error **errp)
 {
     QapiDeallocVisitor *qov = to_qov(v);
     qapi_dealloc_push(qov, obj);
+    return false;
 }

 static void qapi_dealloc_end_struct(Visitor *v)
@@ -76,13 +77,14 @@ static void qapi_dealloc_end_struct(Visitor *v)
     }
 }

-static void qapi_dealloc_start_implicit_struct(Visitor *v,
+static bool qapi_dealloc_start_implicit_struct(Visitor *v,
                                                void **obj,
                                                size_t size,
                                                Error **errp)
 {
     QapiDeallocVisitor *qov = to_qov(v);
     qapi_dealloc_push(qov, obj);
+    return false;
 }

 static void qapi_dealloc_end_implicit_struct(Visitor *v)
@@ -94,10 +96,11 @@ static void qapi_dealloc_end_implicit_struct(Visitor *v)
     }
 }

-static void qapi_dealloc_start_list(Visitor *v, const char *name, Error **errp)
+static bool qapi_dealloc_start_list(Visitor *v, const char *name, Error **errp)
 {
     QapiDeallocVisitor *qov = to_qov(v);
     qapi_dealloc_push(qov, NULL);
+    return false;
 }

 static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList **listp,
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 00f446e..c1abe1e 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -17,10 +17,10 @@
 #include "qapi/visitor.h"
 #include "qapi/visitor-impl.h"

-void visit_start_struct(Visitor *v, void **obj, const char *kind,
+bool visit_start_struct(Visitor *v, void **obj, const char *kind,
                         const char *name, size_t size, Error **errp)
 {
-    v->start_struct(v, obj, kind, name, size, errp);
+    return v->start_struct(v, obj, kind, name, size, errp);
 }

 void visit_check_struct(Visitor *v, Error **errp)
@@ -35,12 +35,13 @@ void visit_end_struct(Visitor *v)
     v->end_struct(v);
 }

-void visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
+bool visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
                                  Error **errp)
 {
     if (v->start_implicit_struct) {
-        v->start_implicit_struct(v, obj, size, errp);
+        return v->start_implicit_struct(v, obj, size, errp);
     }
+    return false;
 }

 void visit_end_implicit_struct(Visitor *v)
@@ -50,9 +51,9 @@ void visit_end_implicit_struct(Visitor *v)
     }
 }

-void visit_start_list(Visitor *v, const char *name, Error **errp)
+bool visit_start_list(Visitor *v, const char *name, Error **errp)
 {
-    v->start_list(v, name, errp);
+    return v->start_list(v, name, errp);
 }

 GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 0270d25..2ad8fd6 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -125,7 +125,7 @@ static void qmp_input_pop(Visitor *v)
     qiv->nb_stack--;
 }

-static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind,
+static bool qmp_input_start_struct(Visitor *v, void **obj, const char *kind,
                                    const char *name, size_t size, Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
@@ -135,30 +135,34 @@ static void qmp_input_start_struct(Visitor *v, void 
**obj, const char *kind,
     if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "QDict");
-        return;
+        return false;
     }

     qmp_input_push(qiv, qobj, &err);
     if (err) {
         error_propagate(errp, err);
-        return;
+        return false;
     }

     if (obj) {
         *obj = g_malloc0(size);
+        return true;
     }
+    return false;
 }


-static void qmp_input_start_implicit_struct(Visitor *v, void **obj,
+static bool qmp_input_start_implicit_struct(Visitor *v, void **obj,
                                             size_t size, Error **errp)
 {
     if (obj) {
         *obj = g_malloc0(size);
+        return true;
     }
+    return false;
 }

-static void qmp_input_start_list(Visitor *v, const char *name, Error **errp)
+static bool qmp_input_start_list(Visitor *v, const char *name, Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
     QObject *qobj = qmp_input_get_object(qiv, name, true);
@@ -166,10 +170,11 @@ static void qmp_input_start_list(Visitor *v, const char 
*name, Error **errp)
     if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "list");
-        return;
+        return false;
     }

     qmp_input_push(qiv, qobj, errp);
+    return true;
 }

 static GenericList *qmp_input_next_list(Visitor *v, GenericList **list,
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index af1e01b..14fdb58 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -111,7 +111,7 @@ static void qmp_output_add_obj(QmpOutputVisitor *qov, const 
char *name,
     }
 }

-static void qmp_output_start_struct(Visitor *v, void **obj, const char *kind,
+static bool qmp_output_start_struct(Visitor *v, void **obj, const char *kind,
                                     const char *name, size_t unused,
                                     Error **errp)
 {
@@ -120,6 +120,7 @@ static void qmp_output_start_struct(Visitor *v, void **obj, 
const char *kind,

     qmp_output_add(qov, name, dict);
     qmp_output_push(qov, dict);
+    return false;
 }

 static void qmp_output_end_struct(Visitor *v)
@@ -128,13 +129,14 @@ static void qmp_output_end_struct(Visitor *v)
     qmp_output_pop(qov);
 }

-static void qmp_output_start_list(Visitor *v, const char *name, Error **errp)
+static bool qmp_output_start_list(Visitor *v, const char *name, Error **errp)
 {
     QmpOutputVisitor *qov = to_qov(v);
     QList *list = qlist_new();

     qmp_output_add(qov, name, list);
     qmp_output_push(qov, list);
+    return false;
 }

 static GenericList *qmp_output_next_list(Visitor *v, GenericList **listp,
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 5b2bc47..6e4dc89 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -117,7 +117,7 @@ error:
     siv->ranges = NULL;
 }

-static void
+static bool
 start_list(Visitor *v, const char *name, Error **errp)
 {
     StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
@@ -131,6 +131,7 @@ start_list(Visitor *v, const char *name, Error **errp)
             siv->cur = r->begin;
         }
     }
+    return true;
 }

 static GenericList *
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index 54d4174..2d9207e 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -259,7 +259,7 @@ static void print_type_number(Visitor *v, double *obj, 
const char *name,
     string_output_set(sov, g_strdup_printf("%f", *obj));
 }

-static void
+static bool
 start_list(Visitor *v, const char *name, Error **errp)
 {
     StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
@@ -268,6 +268,7 @@ start_list(Visitor *v, const char *name, Error **errp)
     assert(sov->list_mode == LM_NONE);
     sov->list_mode = LM_STARTED;
     sov->head = true;
+    return false;
 }

 static GenericList *
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c28208e..eeb6e67 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -58,8 +58,10 @@ def gen_visit_implicit_struct(typ):
 static void visit_type_implicit_%(c_type)s(Visitor *v, %(c_type)s **obj, Error 
**errp)
 {
     Error *err = NULL;
+    bool allocated;

-    visit_start_implicit_struct(v, (void **)obj, sizeof(%(c_type)s), &err);
+    allocated = visit_start_implicit_struct(v, (void **)obj,
+                                            sizeof(%(c_type)s), &err);
     if (err) {
         goto out;
     }
@@ -69,6 +71,10 @@ static void visit_type_implicit_%(c_type)s(Visitor *v, 
%(c_type)s **obj, Error *
     visit_type_%(c_type)s_fields(v, obj, &err);
 out_obj:
     visit_end_implicit_struct(v);
+    if (allocated && err) {
+        g_free(*obj);
+        *obj = NULL;
+    }
 out:
     error_propagate(errp, err);
 }
@@ -116,18 +122,15 @@ out:


 def gen_visit_list(name, element_type):
-    # FIXME: if *obj is NULL on entry, and the first visit_next_list()
-    # assigns to *obj, while a later one fails, we should clean up *obj
-    # rather than leaving it non-NULL. As currently written, the caller must
-    # call qapi_free_FOOList() to avoid a memory leak of the partial FOOList.
     return mcgen('''

 void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, 
Error **errp)
 {
     Error *err = NULL;
     GenericList *i, **prev;
+    bool allocated;

-    visit_start_list(v, name, &err);
+    allocated = visit_start_list(v, name, &err);
     if (err) {
         goto out;
     }
@@ -141,6 +144,10 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, 
const char *name, Error

     visit_end_list(v);
 out:
+    if (allocated && err) {
+        qapi_free_%(c_name)s(*obj);
+        *obj = NULL;
+    }
     error_propagate(errp, err);
 }
 ''',
@@ -171,8 +178,10 @@ def gen_visit_alternate(name, variants):
 void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, 
Error **errp)
 {
     Error *err = NULL;
+    bool allocated;

-    visit_start_implicit_struct(v, (void**) obj, sizeof(%(c_name)s), &err);
+    allocated = visit_start_implicit_struct(v, (void **)obj,
+                                            sizeof(%(c_name)s), &err);
     if (err) {
         goto out;
     }
@@ -201,11 +210,15 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, 
const char *name, Error
     }
 out_obj:
     visit_end_implicit_struct(v);
+    if (allocated && err) {
+        qapi_free_%(c_name)s(*obj);
+        *obj = NULL;
+    }
 out:
     error_propagate(errp, err);
 }
 ''',
-                 name=name)
+                 name=name, c_name=c_name(name))

     return ret

@@ -233,8 +246,10 @@ def gen_visit_object(name, base, members, variants):
 void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, 
Error **errp)
 {
     Error *err = NULL;
+    bool allocated;

-    visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), 
&err);
+    allocated = visit_start_struct(v, (void **)obj, "%(name)s",
+                                   name, sizeof(%(c_name)s), &err);
     if (err) {
         goto out;
     }
@@ -298,10 +313,15 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, 
const char *name, Error
     visit_check_struct(v, &err);
 out_obj:
     visit_end_struct(v);
+    if (allocated && err) {
+        qapi_free_%(c_name)s(*obj);
+        *obj = NULL;
+    }
 out:
     error_propagate(errp, err);
 }
-''')
+''',
+                 c_name=c_name(name))

     return ret

diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index b132775..f03cdf5 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -227,14 +227,13 @@ static void test_dealloc_partial(void)
         QDECREF(ud2_dict);
     }

-    /* verify partial success */
-    assert(ud2 != NULL);
-    assert(ud2->string0 != NULL);
-    assert(strcmp(ud2->string0, text) == 0);
-    assert(ud2->dict1 == NULL);
-
-    /* confirm & release construction error */
+    /* verify that visit_type_XXX() cleans up properly on error */
     error_free_or_abort(&err);
+    assert(!ud2);
+
+    /* Manually create a partial object, leaving ud2->dict1 at NULL */
+    ud2 = g_new0(UserDefTwo, 1);
+    ud2->string0 = g_strdup(text);

     /* tear down partial object */
     qapi_free_UserDefTwo(ud2);
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index f1c2e3b..9792277 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -181,10 +181,7 @@ static void test_validate_fail_struct(TestInputVisitorData 
*data,

     visit_type_TestStruct(v, &p, NULL, &err);
     error_free_or_abort(&err);
-    if (p) {
-        g_free(p->string);
-    }
-    g_free(p);
+    g_assert(!p);
 }

 static void test_validate_fail_struct_nested(TestInputVisitorData *data,
@@ -198,7 +195,7 @@ static void 
test_validate_fail_struct_nested(TestInputVisitorData *data,

     visit_type_UserDefTwo(v, &udp, NULL, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefTwo(udp);
+    g_assert(!udp);
 }

 static void test_validate_fail_list(TestInputVisitorData *data,
@@ -212,7 +209,7 @@ static void test_validate_fail_list(TestInputVisitorData 
*data,

     visit_type_UserDefOneList(v, &head, NULL, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefOneList(head);
+    g_assert(!head);
 }

 static void test_validate_fail_union_native_list(TestInputVisitorData *data,
@@ -227,7 +224,7 @@ static void 
test_validate_fail_union_native_list(TestInputVisitorData *data,

     visit_type_UserDefNativeListUnion(v, &tmp, NULL, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefNativeListUnion(tmp);
+    g_assert(!tmp);
 }

 static void test_validate_fail_union_flat(TestInputVisitorData *data,
@@ -241,7 +238,7 @@ static void 
test_validate_fail_union_flat(TestInputVisitorData *data,

     visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefFlatUnion(tmp);
+    g_assert(!tmp);
 }

 static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData 
*data,
@@ -256,13 +253,13 @@ static void 
test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,

     visit_type_UserDefFlatUnion2(v, &tmp, NULL, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefFlatUnion2(tmp);
+    g_assert(!tmp);
 }

 static void test_validate_fail_alternate(TestInputVisitorData *data,
                                          const void *unused)
 {
-    UserDefAlternate *tmp = NULL;
+    UserDefAlternate *tmp;
     Visitor *v;
     Error *err = NULL;

@@ -270,7 +267,7 @@ static void 
test_validate_fail_alternate(TestInputVisitorData *data,

     visit_type_UserDefAlternate(v, &tmp, NULL, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefAlternate(tmp);
+    g_assert(!tmp);
 }

 static void do_test_validate_qmp_introspect(TestInputVisitorData *data,
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index b4a5bee..45d06f3 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -708,18 +708,12 @@ static void test_visitor_in_errors(TestInputVisitorData 
*data,

     visit_type_TestStruct(v, &p, NULL, &err);
     error_free_or_abort(&err);
-    /* FIXME - a failed parse should not leave a partially-allocated p
-     * for us to clean up; this could cause callers to leak memory. */
-    g_assert(p->string == NULL);
-
-    g_free(p->string);
-    g_free(p);
+    g_assert(!p);

     v = visitor_input_test_init(data, "[ '1', '2', false, '3' ]");
     visit_type_strList(v, &q, NULL, &err);
     error_free_or_abort(&err);
-    assert(q);
-    qapi_free_strList(q);
+    assert(!q);
 }

 static void test_visitor_in_wrong_type(TestInputVisitorData *data,
-- 
2.4.3




reply via email to

[Prev in Thread] Current Thread [Next in Thread]