[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [RFC PATCH 12.5/47] qapi: Document that input visitor seman
From: |
Eric Blake |
Subject: |
[Qemu-devel] [RFC PATCH 12.5/47] qapi: Document that input visitor semantics are prone to leaks |
Date: |
Thu, 30 Jul 2015 16:33:07 -0600 |
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 <address@hidden>
---
As mentioned elsewhere in the thread (comments on 29/47
https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg06107.html)
it would be worth documenting a FIXME for a future series.
I'm submitting this as 12.5/47 due to its relation to other
similar shortcoming doc patches; and assuming Markus can rebase
the rest of the series on top if he wants to fold it into his
v3 posting at this spot. Otherwise, I can wait for his v3 and
rebase it to be part of my (growing) followup series.
scripts/qapi-visit.py | 4 ++++
tests/test-qmp-input-visitor.c | 2 ++
2 files changed, 6 insertions(+)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 73f136f..eec5f1f 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 b7a87ee..a5cfefa 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);
--
2.4.3
- [Qemu-devel] [PATCH RFC v2 00/47] qapi: QMP introspection, Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 03/47] qapi: Simplify guardname(), Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 12/47] tests/qapi-schema: Document events with with base don't work, Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 04/47] qapi-event: Clean up how name of enum QAPIEvent is made, Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 02/47] qapi: Clean up cgen() and mcgen(), Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 09/47] qapi-visit: Replace list implicit_structs by set, Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 07/47] qapi: Generate a nicer struct for flat unions, Markus Armbruster, 2015/07/01