diff options
| author | Eric Blake <eblake@redhat.com> | 2015-07-30 16:33:07 -0600 |
|---|---|---|
| committer | Markus Armbruster <armbru@redhat.com> | 2015-09-04 15:47:14 +0200 |
| commit | 2f52e20597ebd55ede668b2b7d162a84f419b03e (patch) | |
| tree | 661fd001dfeeab007a71e9469e42dae58f0f298d | |
| parent | 999387782f736d7ac0083f4f02e2bc4ce7a9a27b (diff) | |
qapi: Document that input visitor semantics are prone to leaks
Most functions that can return a pointer or set an Error ** value
are decent enough to guarantee a NULL return when reporting an error.
Not so with our generated qapi visitor functions. If the caller
is not careful to clean up partially-allocated objects on error,
then the caller suffers a memory leak.
Properly fixing it is probably complex enough to save for a later
day, so merely document it for now.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1438295587-19069-1-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
| -rw-r--r-- | scripts/qapi-visit.py | 4 | ||||
| -rw-r--r-- | tests/test-qmp-input-visitor.c | 2 |
2 files changed, 6 insertions, 0 deletions
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 73f136fc1d..eec5f1f4c5 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -115,6 +115,10 @@ out: def generate_visit_struct_body(name): + # FIXME: if *obj is NULL on entry, and visit_start_struct() assigns to + # *obj, but then visit_type_FOO_fields() fails, we should clean up *obj + # rather than leaving it non-NULL. As currently written, the caller must + # call qapi_free_FOO() to avoid a memory leak of the partial FOO. ret = mcgen(''' Error *err = NULL; diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c index b7a87ee351..a5cfefae8b 100644 --- a/tests/test-qmp-input-visitor.c +++ b/tests/test-qmp-input-visitor.c @@ -636,6 +636,8 @@ static void test_visitor_in_errors(TestInputVisitorData *data, visit_type_TestStruct(v, &p, NULL, &err); g_assert(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); error_free(err); |