Re: [PATCH] tests: unit: add NULL-pointer check

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> In CID 1432593, Coverity complains that the result of qdict_crumple()
> might leak if it is not a dictionary.  This is not a practical concern
> since the test would fail immediately with a NULL pointer dereference
> in qdict_size().
>
> However, it is not nice to depend on qdict_size() crashing, so add an
> explicit assertion that that the crumpled object was indeed a dictionary.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/unit/check-block-qdict.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/unit/check-block-qdict.c b/tests/unit/check-block-qdict.c
> index 5a25825093..751c58e737 100644
> --- a/tests/unit/check-block-qdict.c
> +++ b/tests/unit/check-block-qdict.c
> @@ -504,7 +504,7 @@ static void qdict_crumple_test_empty(void)
>      src = qdict_new();
>  
>      dst = qobject_to(QDict, qdict_crumple(src, &error_abort));
> -
> +    g_assert(dst);
>      g_assert_cmpint(qdict_size(dst), ==, 0);
>  
>      qobject_unref(src);

First, I'm fine with the patch, so
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Next, permit me a few words on writing tests.  For me, a unit test fails
by crashing.  Crashing with a nice message is optional.  The more likely
the failure, the more useful is niceness.  Complete niceness is
impossible --- if we could predict all crashes, we wouldn't need tests.
Trying to push niceness can be overly verbose.  Thus, judgement calls,
and matters of taste.

Wanting to mollify Coverity is a valid argument.