[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum |
Date: |
Fri, 12 May 2017 08:30:36 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
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?
[...]
- Re: [Qemu-devel] [PATCH 03/17] tests: remove alt num-int cases, (continued)
[Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Marc-André Lureau, 2017/05/09
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Markus Armbruster, 2017/05/11
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Eric Blake, 2017/05/11
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Marc-André Lureau, 2017/05/30
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Eric Blake, 2017/05/30
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Markus Armbruster, 2017/05/30
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Marc-André Lureau, 2017/05/30
Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum,
Markus Armbruster <=
Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Markus Armbruster, 2017/05/12
[Qemu-devel] [PATCH 05/17] qapi: remove promote_int, Marc-André Lureau, 2017/05/09
[Qemu-devel] [PATCH 06/17] qnum: add uint type, Marc-André Lureau, 2017/05/09
[Qemu-devel] [PATCH 07/17] json: learn to parse uint64 numbers, Marc-André Lureau, 2017/05/09