qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: armbru@redhat.com, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: [Qemu-devel] [PATCH v6 01/23] qapi: Make all visitors supply int64/uint64 callbacks
Date: Wed, 25 Nov 2015 17:22:58 -0700	[thread overview]
Message-ID: <1448497401-27784-2-git-send-email-eblake@redhat.com> (raw)
In-Reply-To: <1448497401-27784-1-git-send-email-eblake@redhat.com>

Our qapi visitor contract supports multiple integer visitors:
type_int (64-bit signed; mandatory), type_int64 (64-bit signed,
optional), type_uint64 (64-bit unsigned, optional), and
type_size (64-bit unsigned, optional, with the possibility of
parsing differently than type_uint64 for the case of suffixed
sizes).  But the default code treats any implementation without
type_uint64 as just falling back on type_int, which can cause
confusing results for values larger than LLONG_MAX (such as
having to pass in a negative 2s complement value on input,
and getting a negative result on output).

This patch does not fully address the disparity in parsing,
but does move towards a cleaner situation where EVERY visitor
provides both signed and unsigned variants as entry points;
then each client can either implement sane differences between
the two, or document in place with a FIXME that there is
munging going on for large values.  The next patch will then
clean up the code to no longer allow conditional type_uint64.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v6: new patch, but stems from v5 23/46
---
 qapi/opts-visitor.c          |  5 +++--
 qapi/qapi-dealloc-visitor.c  | 19 ++++++++++---------
 qapi/qmp-input-visitor.c     | 23 ++++++++++++++++++++---
 qapi/qmp-output-visitor.c    | 15 ++++++++++++---
 qapi/string-input-visitor.c  | 22 +++++++++++++++++++---
 qapi/string-output-visitor.c | 16 +++++++++++++---
 6 files changed, 77 insertions(+), 23 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index ef5fb8b..bc2b976 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -354,7 +354,7 @@ opts_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)


 static void
-opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
+opts_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
 {
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
     const QemuOpt *opt;
@@ -522,7 +522,8 @@ opts_visitor_new(const QemuOpts *opts)
      */
     ov->visitor.type_enum = &input_type_enum;

-    ov->visitor.type_int    = &opts_type_int;
+    ov->visitor.type_int    = &opts_type_int64;
+    ov->visitor.type_int64  = &opts_type_int64;
     ov->visitor.type_uint64 = &opts_type_uint64;
     ov->visitor.type_size   = &opts_type_size;
     ov->visitor.type_bool   = &opts_type_bool;
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 737deab..5d1b2e6 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -136,8 +136,13 @@ static void qapi_dealloc_type_str(Visitor *v, char **obj, const char *name,
     }
 }

-static void qapi_dealloc_type_int(Visitor *v, int64_t *obj, const char *name,
-                                  Error **errp)
+static void qapi_dealloc_type_int64(Visitor *v, int64_t *obj,
+                                    const char *name, Error **errp)
+{
+}
+
+static void qapi_dealloc_type_uint64(Visitor *v, uint64_t *obj,
+                                     const char *name, Error **errp)
 {
 }

@@ -159,11 +164,6 @@ static void qapi_dealloc_type_anything(Visitor *v, QObject **obj,
     }
 }

-static void qapi_dealloc_type_size(Visitor *v, uint64_t *obj, const char *name,
-                                   Error **errp)
-{
-}
-
 static void qapi_dealloc_type_enum(Visitor *v, int *obj,
                                    const char * const strings[],
                                    const char *kind, const char *name,
@@ -220,12 +220,13 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
     v->visitor.next_list = qapi_dealloc_next_list;
     v->visitor.end_list = qapi_dealloc_end_list;
     v->visitor.type_enum = qapi_dealloc_type_enum;
-    v->visitor.type_int = qapi_dealloc_type_int;
+    v->visitor.type_int = qapi_dealloc_type_int64;
+    v->visitor.type_int64 = qapi_dealloc_type_int64;
+    v->visitor.type_uint64 = qapi_dealloc_type_uint64;
     v->visitor.type_bool = qapi_dealloc_type_bool;
     v->visitor.type_str = qapi_dealloc_type_str;
     v->visitor.type_number = qapi_dealloc_type_number;
     v->visitor.type_any = qapi_dealloc_type_anything;
-    v->visitor.type_size = qapi_dealloc_type_size;
     v->visitor.start_union = qapi_dealloc_start_union;

     QTAILQ_INIT(&v->stack);
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 932b5f3..96dafcb 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -224,8 +224,23 @@ static void qmp_input_get_next_type(Visitor *v, QType *type, bool promote_int,
     }
 }

-static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name,
-                               Error **errp)
+static void qmp_input_type_int64(Visitor *v, int64_t *obj, const char *name,
+                                 Error **errp)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+    QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true));
+
+    if (!qint) {
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                   "integer");
+        return;
+    }
+
+    *obj = qint_get_int(qint);
+}
+
+static void qmp_input_type_uint64(Visitor *v, uint64_t *obj, const char *name,
+                                  Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
     QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true));
@@ -341,7 +356,9 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
     v->visitor.next_list = qmp_input_next_list;
     v->visitor.end_list = qmp_input_end_list;
     v->visitor.type_enum = input_type_enum;
-    v->visitor.type_int = qmp_input_type_int;
+    v->visitor.type_int = qmp_input_type_int64;
+    v->visitor.type_int64 = qmp_input_type_int64;
+    v->visitor.type_uint64 = qmp_input_type_uint64;
     v->visitor.type_bool = qmp_input_type_bool;
     v->visitor.type_str = qmp_input_type_str;
     v->visitor.type_number = qmp_input_type_number;
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 29899ac..a52f26f 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -158,8 +158,15 @@ static void qmp_output_end_list(Visitor *v, Error **errp)
     qmp_output_pop(qov);
 }

-static void qmp_output_type_int(Visitor *v, int64_t *obj, const char *name,
-                                Error **errp)
+static void qmp_output_type_int64(Visitor *v, int64_t *obj, const char *name,
+                                  Error **errp)
+{
+    QmpOutputVisitor *qov = to_qov(v);
+    qmp_output_add(qov, name, qint_from_int(*obj));
+}
+
+static void qmp_output_type_uint64(Visitor *v, uint64_t *obj, const char *name,
+                                   Error **errp)
 {
     QmpOutputVisitor *qov = to_qov(v);
     qmp_output_add(qov, name, qint_from_int(*obj));
@@ -241,7 +248,9 @@ QmpOutputVisitor *qmp_output_visitor_new(void)
     v->visitor.next_list = qmp_output_next_list;
     v->visitor.end_list = qmp_output_end_list;
     v->visitor.type_enum = output_type_enum;
-    v->visitor.type_int = qmp_output_type_int;
+    v->visitor.type_int = qmp_output_type_int64;
+    v->visitor.type_int64 = qmp_output_type_int64;
+    v->visitor.type_uint64 = qmp_output_type_uint64;
     v->visitor.type_bool = qmp_output_type_bool;
     v->visitor.type_str = qmp_output_type_str;
     v->visitor.type_number = qmp_output_type_number;
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index dee780a..0931321 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -179,8 +179,8 @@ end_list(Visitor *v, Error **errp)
     siv->head = true;
 }

-static void parse_type_int(Visitor *v, int64_t *obj, const char *name,
-                           Error **errp)
+static void parse_type_int64(Visitor *v, int64_t *obj, const char *name,
+                             Error **errp)
 {
     StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);

@@ -221,6 +221,20 @@ error:
                "an int64 value or range");
 }

+static void parse_type_uint64(Visitor *v, uint64_t *obj, const char *name,
+                              Error **errp)
+{
+    /* FIXME - don't handle numbers > INT64_MAX as negative */
+    int64_t i;
+    Error *err = NULL;
+    parse_type_int64(v, &i, name, &err);
+    if (err) {
+        error_propagate(errp, err);
+    } else {
+        *obj = i;
+    }
+}
+
 static void parse_type_size(Visitor *v, uint64_t *obj, const char *name,
                             Error **errp)
 {
@@ -330,7 +344,9 @@ StringInputVisitor *string_input_visitor_new(const char *str)
     v = g_malloc0(sizeof(*v));

     v->visitor.type_enum = input_type_enum;
-    v->visitor.type_int = parse_type_int;
+    v->visitor.type_int = parse_type_int64;
+    v->visitor.type_int64 = parse_type_int64;
+    v->visitor.type_uint64 = parse_type_uint64;
     v->visitor.type_size = parse_type_size;
     v->visitor.type_bool = parse_type_bool;
     v->visitor.type_str = parse_type_str;
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index b86ce2c..83ca6cc 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -116,8 +116,8 @@ static void format_string(StringOutputVisitor *sov, Range *r, bool next,
     }
 }

-static void print_type_int(Visitor *v, int64_t *obj, const char *name,
-                           Error **errp)
+static void print_type_int64(Visitor *v, int64_t *obj, const char *name,
+                             Error **errp)
 {
     StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
     GList *l;
@@ -192,6 +192,14 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
     }
 }

+static void print_type_uint64(Visitor *v, uint64_t *obj, const char *name,
+                             Error **errp)
+{
+    /* FIXME - don't handle numbers > INT64_MAX as negative */
+    int64_t i = *obj;
+    print_type_int64(v, &i, name, errp);
+}
+
 static void print_type_size(Visitor *v, uint64_t *obj, const char *name,
                            Error **errp)
 {
@@ -340,7 +348,9 @@ StringOutputVisitor *string_output_visitor_new(bool human)
     v->string = g_string_new(NULL);
     v->human = human;
     v->visitor.type_enum = output_type_enum;
-    v->visitor.type_int = print_type_int;
+    v->visitor.type_int = print_type_int64;
+    v->visitor.type_int64 = print_type_int64;
+    v->visitor.type_uint64 = print_type_uint64;
     v->visitor.type_size = print_type_size;
     v->visitor.type_bool = print_type_bool;
     v->visitor.type_str = print_type_str;
-- 
2.4.3

  reply	other threads:[~2015-11-26  0:23 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-26  0:22 [Qemu-devel] [PATCH v6 00/23] qapi visitor cleanups (post-introspection cleanups subset E) Eric Blake
2015-11-26  0:22 ` Eric Blake [this message]
2015-11-27 11:17   ` [Qemu-devel] [PATCH v6 01/23] qapi: Make all visitors supply int64/uint64 callbacks Markus Armbruster
2015-11-26  0:22 ` [Qemu-devel] [PATCH v6 02/23] qapi: Require int64/uint64 implementation Eric Blake
2015-11-27 12:05   ` Markus Armbruster
2015-12-02 21:25     ` Eric Blake
2015-12-03  8:30       ` Markus Armbruster
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 03/23] qapi: Consolidate visitor integer callbacks Eric Blake
2015-11-27 12:11   ` Markus Armbruster
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 04/23] qapi: Don't cast Enum* to int* Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 05/23] qmp: Fix reference-counting of qnull on empty output visit Eric Blake
2015-11-27 13:06   ` Markus Armbruster
2015-12-02 23:10     ` Eric Blake
2015-12-03 17:50       ` Markus Armbruster
2015-12-04  3:01         ` Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 06/23] qapi: Don't abuse stack to track qmp-output root Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 07/23] qapi: Document visitor interfaces Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 08/23] qapi: Drop unused error argument for list and implicit struct Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 09/23] hmp: Improve use of qapi visitor Eric Blake
2015-12-04 21:18   ` Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 10/23] vl: " Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 11/23] ppc: Improve use of qapi visitors Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 12/23] balloon: Improve use of qapi visitor Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 13/23] qapi: Add type.is_empty() helper Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 14/23] qapi: Fix command with named empty argument type Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 15/23] qapi: Improve generated event use of qapi visitor Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 16/23] qapi: Track all failures between visit_start/stop Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 17/23] qapi: Eliminate empty visit_type_FOO_fields Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 18/23] qapi: Canonicalize missing object to :empty Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 19/23] qapi-visit: Unify struct and union visit Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 20/23] qapi: Rework deallocation of partial struct Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 21/23] qapi: Simplify extra member error reporting in input visitors Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 22/23] qapi: Split visit_end_struct() into pieces Eric Blake
2015-11-26  0:23 ` [Qemu-devel] [PATCH v6 23/23] qapi: Change visit_type_FOO() to no longer return partial objects Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1448497401-27784-2-git-send-email-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).