Eric Blake <[email protected]> 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 <[email protected]>
>
> ---
> 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);
> }
[...]
