Skip to content

Commit

Permalink
qapi: Document that input visitor semantics are prone to leaks
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
ebblake authored and Markus Armbruster committed Sep 4, 2015
1 parent 9993877 commit 2f52e20
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 0 deletions.
4 changes: 4 additions & 0 deletions scripts/qapi-visit.py
Expand Up @@ -115,6 +115,10 @@ def generate_visit_struct_fields(name, members, base = None):


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;
Expand Down
2 changes: 2 additions & 0 deletions tests/test-qmp-input-visitor.c
Expand Up @@ -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);
Expand Down

0 comments on commit 2f52e20

Please sign in to comment.