Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum

[Markus Armbruster]

Question for Luiz...

Marc-André Lureau <address@hidden> writes:

[...]
> diff --git a/tests/check-qnum.c b/tests/check-qnum.c
> new file mode 100644
> index 0000000000..d08d35e85a
> --- /dev/null
> +++ b/tests/check-qnum.c
> @@ -0,0 +1,131 @@
> +/*
> + * QNum unit-tests.
> + *
> + * Copyright (C) 2009 Red Hat Inc.
> + *
> + * Authors:
> + *  Luiz Capitulino <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or 
> later.
> + * See the COPYING.LIB file in the top-level directory.
> + */
> +#include "qemu/osdep.h"
> +
> +#include "qapi/qmp/qnum.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +
> +/*
> + * Public Interface test-cases
> + *
> + * (with some violations to access 'private' data)
> + */
> +
> +static void qnum_from_int_test(void)
> +{
> +    QNum *qi;
> +    const int value = -42;
> +
> +    qi = qnum_from_int(value);
> +    g_assert(qi != NULL);
> +    g_assert_cmpint(qi->u.i64, ==, value);
> +    g_assert_cmpint(qi->base.refcnt, ==, 1);
> +    g_assert_cmpint(qobject_type(QOBJECT(qi)), ==, QTYPE_QNUM);
> +
> +    // destroy doesn't exit yet
> +    g_free(qi);
> +}

The comment is enigmatic.  It was first written in commit 33837ba
"Introduce QInt unit-tests", and got copied around since.  In
check-qlist.c, it's spelled "exist yet".

What is "destroy", why doesn't it exit / exist now, but will exit /
exist later?  It can't be qnum_destroy_obj(), because that certainly
exists already, exits already in the sense of returning, and shouldn't
ever exit in the sense of terminating the program.

The comment applies to a g_free().  Why do we free directly instead
decrementing the reference count?  Perhaps the comment tries to explain
that (if it does, it fails).

Luiz, any idea?

[...]