diff mbox

[1/2] qobject: Use 'bool' for qbool

Message ID 1431728700-18055-2-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake May 15, 2015, 10:24 p.m. UTC
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 <eblake@redhat.com>
---
 block/qapi.c                    |  2 +-
 block/quorum.c                  |  4 ++--
 block/vvfat.c                   |  4 ++--
 include/qapi/qmp/qbool.h        |  8 ++++----
 monitor.c                       | 10 +++++-----
 qapi/qmp-input-visitor.c        |  2 +-
 qapi/qmp-output-visitor.c       |  2 +-
 qobject/json-parser.c           |  6 +++---
 qobject/qbool.c                 |  8 ++++----
 qobject/qdict.c                 |  4 ++--
 qobject/qjson.c                 |  2 +-
 qom/object.c                    |  4 ++--
 tests/check-qjson.c             | 11 ++++++-----
 tests/test-qmp-event.c          |  4 ++--
 tests/test-qmp-output-visitor.c |  4 ++--
 util/qemu-option.c              |  2 +-
 16 files changed, 39 insertions(+), 38 deletions(-)

Comments

Andreas Färber May 16, 2015, 1:30 p.m. UTC | #1
Am 16.05.2015 um 00:24 schrieb Eric Blake:
> 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 <eblake@redhat.com>
> ---
>  block/qapi.c                    |  2 +-
>  block/quorum.c                  |  4 ++--
>  block/vvfat.c                   |  4 ++--
>  include/qapi/qmp/qbool.h        |  8 ++++----
>  monitor.c                       | 10 +++++-----
>  qapi/qmp-input-visitor.c        |  2 +-
>  qapi/qmp-output-visitor.c       |  2 +-
>  qobject/json-parser.c           |  6 +++---
>  qobject/qbool.c                 |  8 ++++----
>  qobject/qdict.c                 |  4 ++--
>  qobject/qjson.c                 |  2 +-
>  qom/object.c                    |  4 ++--
>  tests/check-qjson.c             | 11 ++++++-----
>  tests/test-qmp-event.c          |  4 ++--
>  tests/test-qmp-output-visitor.c |  4 ++--
>  util/qemu-option.c              |  2 +-
>  16 files changed, 39 insertions(+), 38 deletions(-)
[...]
> diff --git a/qobject/qbool.c b/qobject/qbool.c
> index a3d2afa..5ff69f0 100644
> --- a/qobject/qbool.c
> +++ b/qobject/qbool.c
> @@ -23,11 +23,11 @@ static const QType qbool_type = {
>  };
> 
>  /**
> - * qbool_from_int(): Create a new QBool from an int
> + * qbool_from_bool(): Create a new QBool from a bool
>   *
>   * Return strong reference.

Can you fix the syntax as follow-up please?

/**
 * qbool_from_bool:
 * @value: ...
 *
 * Desc...
 *
 * Returns: ...
 */

>   */
> -QBool *qbool_from_int(int value)
> +QBool *qbool_from_bool(bool value)
>  {
>      QBool *qb;
> 
> @@ -39,9 +39,9 @@ QBool *qbool_from_int(int value)
>  }
> 
>  /**
> - * qbool_get_int(): Get the stored int
> + * qbool_get_bool(): Get the stored bool
>   */
> -int qbool_get_int(const QBool *qb)
> +bool qbool_get_bool(const QBool *qb)
>  {
>      return qb->value;
>  }
[...]
> 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)
>      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);
> 
>      qbool = qobject_to_qbool(obj);
> -    g_assert(qbool_get_int(qbool) != 0);
> +    g_assert(qbool_get_bool(qbool) == true);
> 
>      QDECREF(qbool);
> 
[...]
> diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
> index f8c9367..5be8e77 100644
> --- a/tests/test-qmp-output-visitor.c
> +++ b/tests/test-qmp-output-visitor.c
> @@ -72,7 +72,7 @@ static void test_visitor_out_bool(TestOutputVisitorData *data,
>      obj = qmp_output_get_qobject(data->qov);
>      g_assert(obj != NULL);
>      g_assert(qobject_type(obj) == QTYPE_QBOOL);
> -    g_assert(qbool_get_int(qobject_to_qbool(obj)) == value);
> +    g_assert(qbool_get_bool(qobject_to_qbool(obj)) == value);
> 
>      qobject_decref(obj);
>  }
> @@ -662,7 +662,7 @@ static void check_native_list(QObject *qobj,
>              tmp = qlist_peek(qlist);
>              g_assert(tmp);
>              qvalue = qobject_to_qbool(tmp);
> -            g_assert_cmpint(qbool_get_int(qvalue), ==, (i % 3 == 0) ? 1 : 0);
> +            g_assert_cmpint(qbool_get_bool(qvalue), ==, i % 3 == 0);
>              qobject_decref(qlist_pop(qlist));
>          }
>          break;
[snip]

I notice that we're inconsistent in using g_assert() vs.
g_assert_cmpint(). Given that GLib has a weird GBoolean, should we add a
macro qtest_assert_cmpbool() instead as follow-up?

That said,

Reviewed-by: Andreas Färber <afaerber@suse.de>

Regards,
Andreas
Eric Blake May 16, 2015, 5:13 p.m. UTC | #2
On 05/16/2015 07:30 AM, Andreas Färber wrote:
> Am 16.05.2015 um 00:24 schrieb Eric Blake:
>> 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 <eblake@redhat.com>
>> ---

>>  /**
>> - * qbool_from_int(): Create a new QBool from an int
>> + * qbool_from_bool(): Create a new QBool from a bool
>>   *
>>   * Return strong reference.
> 
> Can you fix the syntax as follow-up please?
> 
> /**
>  * qbool_from_bool:
>  * @value: ...
>  *
>  * Desc...
>  *
>  * Returns: ...
>  */

Sure, I can do that over all the qboject files, as a new patch.

>> @@ -662,7 +662,7 @@ static void check_native_list(QObject *qobj,
>>              tmp = qlist_peek(qlist);
>>              g_assert(tmp);
>>              qvalue = qobject_to_qbool(tmp);
>> -            g_assert_cmpint(qbool_get_int(qvalue), ==, (i % 3 == 0) ? 1 : 0);
>> +            g_assert_cmpint(qbool_get_bool(qvalue), ==, i % 3 == 0);
>>              qobject_decref(qlist_pop(qlist));
>>          }
>>          break;
> [snip]
> 
> I notice that we're inconsistent in using g_assert() vs.
> g_assert_cmpint(). Given that GLib has a weird GBoolean, should we add a
> macro qtest_assert_cmpbool() instead as follow-up?

We aren't even touching GBoolean (qbool_get_bool now returns 'bool', not
GBoolean; and bool promotes just fine to int under C rules), so I don't
see the point to making any further changes here.  Or are you proposing
that the new macro would do something like 'expecting "true" but got
"false"' instead of g_assert_cmpint() collapsing things to 0 and 1?

> 
> That said,
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> 
> Regards,
> Andreas
>
Markus Armbruster May 18, 2015, 6:42 a.m. UTC | #3
Eric Blake <eblake@redhat.com> writes:

> On 05/16/2015 07:30 AM, Andreas Färber wrote:
>> Am 16.05.2015 um 00:24 schrieb Eric Blake:
>>> 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 <eblake@redhat.com>
>>> ---
>
>>>  /**
>>> - * qbool_from_int(): Create a new QBool from an int
>>> + * qbool_from_bool(): Create a new QBool from a bool
>>>   *
>>>   * Return strong reference.
      */
>> 
>> Can you fix the syntax as follow-up please?
>> 
>> /**
>>  * qbool_from_bool:
>>  * @value: ...
>>  *
>>  * Desc...
>>  *
>>  * Returns: ...
>>  */
>
> Sure, I can do that over all the qboject files, as a new patch.

Please don't, it's an egregious waste of screen space and the reader's
mental energy.

We're not using GTK-Doc for anything, and as long as we don't, I'm
unwilling to pay its price of admission.

If a maintainer prefers to enforce GTK-Doc comment syntax in his
subsystem, I won't argue.  In the (few) places I maintain, I'll insist
on readable, concise comments.  The @sigils are welcome, repeating
obvious things and other waste of space is not.  For what it's worth,
Kevin shared this sentiment last time we discussed it.

Luiz's call, because he's the maintainer.

>>> @@ -662,7 +662,7 @@ static void check_native_list(QObject *qobj,
>>>              tmp = qlist_peek(qlist);
>>>              g_assert(tmp);
>>>              qvalue = qobject_to_qbool(tmp);
>>> -            g_assert_cmpint(qbool_get_int(qvalue), ==, (i % 3 == 0) ? 1 : 0);
>>> +            g_assert_cmpint(qbool_get_bool(qvalue), ==, i % 3 == 0);
>>>              qobject_decref(qlist_pop(qlist));
>>>          }
>>>          break;
>> [snip]
>> 
>> I notice that we're inconsistent in using g_assert() vs.
>> g_assert_cmpint(). Given that GLib has a weird GBoolean, should we add a
>> macro qtest_assert_cmpbool() instead as follow-up?
>
> We aren't even touching GBoolean (qbool_get_bool now returns 'bool', not
> GBoolean; and bool promotes just fine to int under C rules), so I don't
> see the point to making any further changes here.  Or are you proposing
> that the new macro would do something like 'expecting "true" but got
> "false"' instead of g_assert_cmpint() collapsing things to 0 and 1?

Promoting to int here is just fine with me.

Andreas is right on GBoolean of course.  It's a relic that has become a
trap for the unwary.  Let's stay away from it as much as we can.
Alberto Garcia May 19, 2015, 12:59 p.m. UTC | #4
On Sat 16 May 2015 12:24:59 AM CEST, Eric Blake <eblake@redhat.com> wrote:
> 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 <eblake@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto
Markus Armbruster June 12, 2015, 5:35 a.m. UTC | #5
Patch looks good to me, but it made me wonder about something.  Please
find the question inline.

Eric Blake <eblake@redhat.com> 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 <eblake@redhat.com>
[...]
> 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);
>
[...]
Eric Blake June 12, 2015, 3:38 p.m. UTC | #6
On 06/11/2015 11:35 PM, Markus Armbruster wrote:
> Patch looks good to me, but it made me wonder about something.  Please
> find the question inline.
> 
> Eric Blake <eblake@redhat.com> 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.
>>

>> @@ -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?
> 

When using va_arg(), you have to use the type argument that things
promote to when called through var-args (that is, va_arg(*ap, bool)
would be a compiler warning).

I don't know if anyone besides the testsuite is using %i; but I've
already mentioned in another thread [1] that the correlation between...

>>
>> -    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".

...qobject_from_json in one file to the implementation of %i in another
was very hard to trace when writing this patch, as well as the idea of
getting rid of the remaining 2 out of only 3 clients of %p [1].  So it's
already on my table of ideas to do a followup patch that adds
documentation, and audits all clients to see what can be pruned.

[1] https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg04660.html
diff mbox

Patch

diff --git a/block/qapi.c b/block/qapi.c
index 18d2b95..666bcb2 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -514,7 +514,7 @@  static void dump_qobject(fprintf_function func_fprintf, void *f,
         }
         case QTYPE_QBOOL: {
             QBool *value = qobject_to_qbool(obj);
-            func_fprintf(f, "%s", qbool_get_int(value) ? "true" : "false");
+            func_fprintf(f, "%s", qbool_get_bool(value) ? "true" : "false");
             break;
         }
         case QTYPE_QERROR: {
diff --git a/block/quorum.c b/block/quorum.c
index f91ef75..0e419ac 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1053,9 +1053,9 @@  static void quorum_refresh_filename(BlockDriverState *bs)
     qdict_put_obj(opts, QUORUM_OPT_VOTE_THRESHOLD,
                   QOBJECT(qint_from_int(s->threshold)));
     qdict_put_obj(opts, QUORUM_OPT_BLKVERIFY,
-                  QOBJECT(qbool_from_int(s->is_blkverify)));
+                  QOBJECT(qbool_from_bool(s->is_blkverify)));
     qdict_put_obj(opts, QUORUM_OPT_REWRITE,
-                  QOBJECT(qbool_from_int(s->rewrite_corrupted)));
+                  QOBJECT(qbool_from_bool(s->rewrite_corrupted)));
     qdict_put_obj(opts, "children", QOBJECT(children));

     bs->full_open_options = opts;
diff --git a/block/vvfat.c b/block/vvfat.c
index e803589..f3e3d49 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1059,8 +1059,8 @@  static void vvfat_parse_filename(const char *filename, QDict *options,
     /* Fill in the options QDict */
     qdict_put(options, "dir", qstring_from_str(filename));
     qdict_put(options, "fat-type", qint_from_int(fat_type));
-    qdict_put(options, "floppy", qbool_from_int(floppy));
-    qdict_put(options, "rw", qbool_from_int(rw));
+    qdict_put(options, "floppy", qbool_from_bool(floppy));
+    qdict_put(options, "rw", qbool_from_bool(rw));
 }

 static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
diff --git a/include/qapi/qmp/qbool.h b/include/qapi/qmp/qbool.h
index c4eaab9..4aa6be3 100644
--- a/include/qapi/qmp/qbool.h
+++ b/include/qapi/qmp/qbool.h
@@ -14,16 +14,16 @@ 
 #ifndef QBOOL_H
 #define QBOOL_H

-#include <stdint.h>
+#include <stdbool.h>
 #include "qapi/qmp/qobject.h"

 typedef struct QBool {
     QObject_HEAD;
-    int value;
+    bool value;
 } QBool;

-QBool *qbool_from_int(int value);
-int qbool_get_int(const QBool *qb);
+QBool *qbool_from_bool(bool value);
+bool qbool_get_bool(const QBool *qb);
 QBool *qobject_to_qbool(const QObject *obj);

 #endif /* QBOOL_H */
diff --git a/monitor.c b/monitor.c
index b2561e1..c17d874 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3986,7 +3986,7 @@  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
         case 'b':
             {
                 const char *beg;
-                int val;
+                bool val;

                 while (qemu_isspace(*p)) {
                     p++;
@@ -3996,14 +3996,14 @@  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                     p++;
                 }
                 if (p - beg == 2 && !memcmp(beg, "on", p - beg)) {
-                    val = 1;
+                    val = true;
                 } else if (p - beg == 3 && !memcmp(beg, "off", p - beg)) {
-                    val = 0;
+                    val = false;
                 } else {
                     monitor_printf(mon, "Expected 'on' or 'off'\n");
                     goto fail;
                 }
-                qdict_put(qdict, key, qbool_from_int(val));
+                qdict_put(qdict, key, qbool_from_bool(val));
             }
             break;
         case '-':
@@ -4034,7 +4034,7 @@  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                     } else {
                         /* has option */
                         p++;
-                        qdict_put(qdict, key, qbool_from_int(1));
+                        qdict_put(qdict, key, qbool_from_bool(true));
                     }
                 }
             }
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index d861206..f6dab1a 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -248,7 +248,7 @@  static void qmp_input_type_bool(Visitor *v, bool *obj, const char *name,
         return;
     }

-    *obj = qbool_get_int(qobject_to_qbool(qobj));
+    *obj = qbool_get_bool(qobject_to_qbool(qobj));
 }

 static void qmp_input_type_str(Visitor *v, char **obj, const char *name,
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 96b3384..7e0f7ce 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -166,7 +166,7 @@  static void qmp_output_type_bool(Visitor *v, bool *obj, const char *name,
                                  Error **errp)
 {
     QmpOutputVisitor *qov = to_qov(v);
-    qmp_output_add(qov, name, qbool_from_int(*obj));
+    qmp_output_add(qov, name, qbool_from_bool(*obj));
 }

 static void qmp_output_type_str(Visitor *v, char **obj, const char *name,
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)));
     } 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/qobject/qbool.c b/qobject/qbool.c
index a3d2afa..5ff69f0 100644
--- a/qobject/qbool.c
+++ b/qobject/qbool.c
@@ -23,11 +23,11 @@  static const QType qbool_type = {
 };

 /**
- * qbool_from_int(): Create a new QBool from an int
+ * qbool_from_bool(): Create a new QBool from a bool
  *
  * Return strong reference.
  */
-QBool *qbool_from_int(int value)
+QBool *qbool_from_bool(bool value)
 {
     QBool *qb;

@@ -39,9 +39,9 @@  QBool *qbool_from_int(int value)
 }

 /**
- * qbool_get_int(): Get the stored int
+ * qbool_get_bool(): Get the stored bool
  */
-int qbool_get_int(const QBool *qb)
+bool qbool_get_bool(const QBool *qb)
 {
     return qb->value;
 }
diff --git a/qobject/qdict.c b/qobject/qdict.c
index ea239f0..d181969 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -244,7 +244,7 @@  int64_t qdict_get_int(const QDict *qdict, const char *key)
 int qdict_get_bool(const QDict *qdict, const char *key)
 {
     QObject *obj = qdict_get_obj(qdict, key, QTYPE_QBOOL);
-    return qbool_get_int(qobject_to_qbool(obj));
+    return qbool_get_bool(qobject_to_qbool(obj));
 }

 /**
@@ -322,7 +322,7 @@  int qdict_get_try_bool(const QDict *qdict, const char *key, int def_value)
     if (!obj || qobject_type(obj) != QTYPE_QBOOL)
         return def_value;

-    return qbool_get_int(qobject_to_qbool(obj));
+    return qbool_get_bool(qobject_to_qbool(obj));
 }

 /**
diff --git a/qobject/qjson.c b/qobject/qjson.c
index 846733d..f022edc 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -254,7 +254,7 @@  static void to_json(const QObject *obj, QString *str, int pretty, int indent)
     case QTYPE_QBOOL: {
         QBool *val = qobject_to_qbool(obj);

-        if (qbool_get_int(val)) {
+        if (qbool_get_bool(val)) {
             qstring_append(str, "true");
         } else {
             qstring_append(str, "false");
diff --git a/qom/object.c b/qom/object.c
index b8dff43..0b46df6 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -901,7 +901,7 @@  Object *object_property_get_link(Object *obj, const char *name,
 void object_property_set_bool(Object *obj, bool value,
                               const char *name, Error **errp)
 {
-    QBool *qbool = qbool_from_int(value);
+    QBool *qbool = qbool_from_bool(value);
     object_property_set_qobject(obj, QOBJECT(qbool), name, errp);

     QDECREF(qbool);
@@ -922,7 +922,7 @@  bool object_property_get_bool(Object *obj, const char *name,
         error_set(errp, QERR_INVALID_PARAMETER_TYPE, name, "boolean");
         retval = false;
     } else {
-        retval = qbool_get_int(qbool);
+        retval = qbool_get_bool(qbool);
     }

     QDECREF(qbool);
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)
     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);

     qbool = qobject_to_qbool(obj);
-    g_assert(qbool_get_int(qbool) != 0);
+    g_assert(qbool_get_bool(qbool) == true);

     QDECREF(qbool);

diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
index cb354e6..1ee40e1 100644
--- a/tests/test-qmp-event.c
+++ b/tests/test-qmp-event.c
@@ -60,8 +60,8 @@  void qdict_cmp_do_simple(const char *key, QObject *obj1, void *opaque)

     switch (qobject_type(obj1)) {
     case QTYPE_QBOOL:
-        d->result = (qbool_get_int(qobject_to_qbool(obj1)) ==
-                     qbool_get_int(qobject_to_qbool(obj2)));
+        d->result = (qbool_get_bool(qobject_to_qbool(obj1)) ==
+                     qbool_get_bool(qobject_to_qbool(obj2)));
         return;
     case QTYPE_QINT:
         d->result = (qint_get_int(qobject_to_qint(obj1)) ==
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index f8c9367..5be8e77 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -72,7 +72,7 @@  static void test_visitor_out_bool(TestOutputVisitorData *data,
     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
     g_assert(qobject_type(obj) == QTYPE_QBOOL);
-    g_assert(qbool_get_int(qobject_to_qbool(obj)) == value);
+    g_assert(qbool_get_bool(qobject_to_qbool(obj)) == value);

     qobject_decref(obj);
 }
@@ -662,7 +662,7 @@  static void check_native_list(QObject *qobj,
             tmp = qlist_peek(qlist);
             g_assert(tmp);
             qvalue = qobject_to_qbool(tmp);
-            g_assert_cmpint(qbool_get_int(qvalue), ==, (i % 3 == 0) ? 1 : 0);
+            g_assert_cmpint(qbool_get_bool(qvalue), ==, i % 3 == 0);
             qobject_decref(qlist_pop(qlist));
         }
         break;
diff --git a/util/qemu-option.c b/util/qemu-option.c
index fda4e5f..8a2e8e5 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -919,7 +919,7 @@  static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
         break;
     case QTYPE_QBOOL:
         pstrcpy(buf, sizeof(buf),
-                qbool_get_int(qobject_to_qbool(obj)) ? "on" : "off");
+                qbool_get_bool(qobject_to_qbool(obj)) ? "on" : "off");
         value = buf;
         break;
     default: