[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 05/30] qapi: Plug leaks in test-qmp-*
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v10 05/30] qapi: Plug leaks in test-qmp-* |
Date: |
Fri, 06 Nov 2015 16:21:50 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> Make valgrind happy with the current state of the tests, so that
> it is easier to see if future patches introduce new memory problems
> without being drowned in noise. Many of the leaks were due to
> calling a second init without tearing down the data from an earlier
> visit. But since teardown is already idempotent, and we already
> register teardown as part of input_visitor_test_add(), it is nicer
> to just make init() safe to call multiple times than it is to have
> to make all tests call teardown.
>
> Another common leak was forgetting to clean up an error object,
> after testing that an error was raised.
>
> Another leak was in test_visitor_in_struct_nested(), failing to
> clean the base member of UserDefTwo. Cleaning that up left
> check_and_free_str() as dead code (since using the qapi_free_*
> takes care of recursion, and we don't want double frees).
>
> A final leak was in test_visitor_out_any(), which was reassigning
> the qobj local variable to a subset of the overall structure
> needing freeing; it did not result in a use-after-free, but
> was not cleaning up all the qdict.
>
> test-qmp-event and test-qmp-commands were already clean.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v10: improve commit message, split out refactor of test init so
> that both init and init_raw benefit from change
> v9: move earlier in series (was 13/17)
> v8: no change
> v7: no change
> v6: make init repeatable rather than adding teardown everywhere,
> fix additional leak with UserDefTwo base, plug additional files
> ---
> tests/test-qmp-input-strict.c | 9 +++++++++
> tests/test-qmp-input-visitor.c | 41
> +++++++----------------------------------
> tests/test-qmp-output-visitor.c | 3 ++-
> 3 files changed, 18 insertions(+), 35 deletions(-)
>
> diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
> index 77151de..95cd8e1 100644
> --- a/tests/test-qmp-input-strict.c
> +++ b/tests/test-qmp-input-strict.c
> @@ -49,6 +49,8 @@ static Visitor
> *validate_test_init_internal(TestInputVisitorData *data,
> {
> Visitor *v;
>
> + validate_teardown(data, NULL);
> +
> data->obj = qobject_from_jsonv(json_string, ap);
> g_assert(data->obj);
>
> @@ -191,6 +193,7 @@ static void
> test_validate_fail_struct(TestInputVisitorData *data,
>
> visit_type_TestStruct(v, &p, NULL, &err);
> g_assert(err);
> + error_free(err);
v9 had /* FIXME: visitor should not allocate p when returning error */
here. Did you drop it intentionally?
If you put it back, please mention it in the commit message.
> if (p) {
> g_free(p->string);
> }
[...]
- [Qemu-devel] [PATCH v10 02/30] qapi: Strengthen test of TestStructList, (continued)
- [Qemu-devel] [PATCH v10 02/30] qapi: Strengthen test of TestStructList, Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 04/30] qapi: Share test_init code in test-qmp-input*, Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 01/30] qapi: Use generated TestStruct machinery in tests, Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 14/30] qapi-types: Consolidate gen_struct() and gen_union(), Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 12/30] qapi-introspect: Document lack of sorting, Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 05/30] qapi: Plug leaks in test-qmp-*, Eric Blake, 2015/11/06
- Re: [Qemu-devel] [PATCH v10 05/30] qapi: Plug leaks in test-qmp-*,
Markus Armbruster <=
- [Qemu-devel] [PATCH v10 15/30] qapi-types: Simplify gen_struct_field[s], Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 16/30] qapi: Drop obsolete tag value collision assertions, Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 11/30] qapi: Provide nicer array names in introspection, Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 07/30] qapi: Simplify error cleanup in test-qmp-*, Eric Blake, 2015/11/06
[Qemu-devel] [PATCH] fixup! qapi: Simplify error cleanup in test-qmp-*, Eric Blake, 2015/11/06