Re: [Qemu-devel] [PATCH 1/2] qobject: Use 'bool' for qbool

[Markus Armbruster]

Patch looks good to me, but it made me wonder about something.  Please
find the question inline.

Eric Blake <address@hidden> writes:

> We require a C99 compiler, so let's use 'bool' instead of 'int'
> when dealing with boolean values.  There are few enough clients
> to fix them all in one pass.
>
> Signed-off-by: Eric Blake <address@hidden>
[...]
> diff --git a/qobject/json-parser.c b/qobject/json-parser.c
> index 717cb8f..015d785 100644
> --- a/qobject/json-parser.c
> +++ b/qobject/json-parser.c
> @@ -558,9 +558,9 @@ static QObject *parse_keyword(JSONParserContext *ctxt)
>      }
>
>      if (token_is_keyword(token, "true")) {
> -        ret = QOBJECT(qbool_from_int(true));
> +        ret = QOBJECT(qbool_from_bool(true));
>      } else if (token_is_keyword(token, "false")) {
> -        ret = QOBJECT(qbool_from_int(false));
> +        ret = QOBJECT(qbool_from_bool(false));
>      } else if (token_is_keyword(token, "null")) {
>          ret = qnull();
>      } else {
> @@ -593,7 +593,7 @@ static QObject *parse_escape(JSONParserContext *ctxt, 
> va_list *ap)
>      if (token_is_escape(token, "%p")) {
>          obj = va_arg(*ap, QObject *);
>      } else if (token_is_escape(token, "%i")) {
> -        obj = QOBJECT(qbool_from_int(va_arg(*ap, int)));
> +        obj = QOBJECT(qbool_from_bool(va_arg(*ap, int)));

Funny: JSON_ESCAPE "%i" gets an int, but maps it to bool.  See also
patch to check-qjson.c below.

Is this feature actually used anywhere other than the tests?

>      } else if (token_is_escape(token, "%d")) {
>          obj = QOBJECT(qint_from_int(va_arg(*ap, int)));
>      } else if (token_is_escape(token, "%ld")) {
[...]
> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
> index 60e5b22..1cfffa5 100644
> --- a/tests/check-qjson.c
> +++ b/tests/check-qjson.c
> @@ -1013,7 +1013,7 @@ static void keyword_literal(void)
>      g_assert(qobject_type(obj) == QTYPE_QBOOL);
>
>      qbool = qobject_to_qbool(obj);
> -    g_assert(qbool_get_int(qbool) != 0);
> +    g_assert(qbool_get_bool(qbool) == true);
>
>      str = qobject_to_json(obj);
>      g_assert(strcmp(qstring_get_str(str), "true") == 0);
> @@ -1026,7 +1026,7 @@ static void keyword_literal(void)
>      g_assert(qobject_type(obj) == QTYPE_QBOOL);
>
>      qbool = qobject_to_qbool(obj);
> -    g_assert(qbool_get_int(qbool) == 0);
> +    g_assert(qbool_get_bool(qbool) == false);
>
>      str = qobject_to_json(obj);
>      g_assert(strcmp(qstring_get_str(str), "false") == 0);
> @@ -1039,16 +1039,17 @@ static void keyword_literal(void)
       obj = qobject_from_jsonf("%i", false);
       g_assert(obj != NULL);
>      g_assert(qobject_type(obj) == QTYPE_QBOOL);
>
>      qbool = qobject_to_qbool(obj);
> -    g_assert(qbool_get_int(qbool) == 0);
> +    g_assert(qbool_get_bool(qbool) == false);
>
>      QDECREF(qbool);
>
> -    obj = qobject_from_jsonf("%i", true);
> +    /* Test that non-zero values other than 1 get collapsed to true */
> +    obj = qobject_from_jsonf("%i", 2);
>      g_assert(obj != NULL);
>      g_assert(qobject_type(obj) == QTYPE_QBOOL);

These are test test cases for JSON_ESCAPE "%i".

>
>      qbool = qobject_to_qbool(obj);
> -    g_assert(qbool_get_int(qbool) != 0);
> +    g_assert(qbool_get_bool(qbool) == true);
>
>      QDECREF(qbool);
>
[...]