diff mbox

[v6,03/23] qapi: Consolidate visitor integer callbacks

Message ID 1448497401-27784-4-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake Nov. 26, 2015, 12:23 a.m. UTC 
Commit 4e27e819 introduced optional visitor callbacks for all
sorts of int types, but except for type_uint64() and type_size(),
none of them have ever been supplied (the generic implementation
based on using type_[u]int64() then bounds-checking works just
fine). In the interest of simplicity, it's easier to make the
visitor callback interface not have to worry about the other
sizes.

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

---
v6: split off from v5 23/46
original version also appeared in v6-v9 of subset D
---
 include/qapi/visitor-impl.h |  22 ++++-----
 qapi/qapi-visit-core.c      | 117 ++++++++++++++++++--------------------------
 2 files changed, 59 insertions(+), 80 deletions(-)

Comments

Markus Armbruster Nov. 27, 2015, 12:11 p.m. UTC | #1 
Eric Blake <eblake@redhat.com> writes:

> Commit 4e27e819 introduced optional visitor callbacks for all
> sorts of int types, but except for type_uint64() and type_size(),
> none of them have ever been supplied (the generic implementation
> based on using type_[u]int64() then bounds-checking works just
> fine). In the interest of simplicity, it's easier to make the
> visitor callback interface not have to worry about the other
> sizes.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> v6: split off from v5 23/46
> original version also appeared in v6-v9 of subset D
> ---
>  include/qapi/visitor-impl.h |  22 ++++-----
>  qapi/qapi-visit-core.c      | 117 ++++++++++++++++++--------------------------
>  2 files changed, 59 insertions(+), 80 deletions(-)
>
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 70326e0..d071c06 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -1,7 +1,7 @@
>  /*
>   * Core Definitions for QAPI Visitor implementations
>   *
> - * Copyright (C) 2012 Red Hat, Inc.
> + * Copyright (C) 2012, 2015 Red Hat, Inc.
>   *
>   * Author: Paolo Bonizni <pbonzini@redhat.com>
>   *
> @@ -36,6 +36,16 @@ struct Visitor
>      void (*get_next_type)(Visitor *v, QType *type, bool promote_int,
>                            const char *name, Error **errp);
>
> +    /* Must be set. */
> +    void (*type_int64)(Visitor *v, int64_t *obj, const char *name,
> +                       Error **errp);
> +    /* Must be set. */
> +    void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name,
> +                        Error **errp);
> +    /* Only required to visit sizes differently than (*type_uint64)().  */

What about:

       /* Optional; fallback is type_uint64() */

> +    void (*type_size)(Visitor *v, uint64_t *obj, const char *name,
> +                      Error **errp);
> +    /* Must be set. */
>      void (*type_bool)(Visitor *v, bool *obj, const char *name, Error **errp);
>      void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
>      void (*type_number)(Visitor *v, double *obj, const char *name,
> @@ -46,16 +56,6 @@ struct Visitor
>      /* May be NULL; most useful for input visitors. */
>      void (*optional)(Visitor *v, bool *present, const char *name);
>
> -    void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp);
> -    void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
> -    void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp);
> -    void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
> -    void (*type_int8)(Visitor *v, int8_t *obj, const char *name, Error **errp);
> -    void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error **errp);
> -    void (*type_int32)(Visitor *v, int32_t *obj, const char *name, Error **errp);
> -    void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
> -    /* visit_type_size() falls back to (*type_uint64)() if type_size is unset */
> -    void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>      bool (*start_union)(Visitor *v, bool data_present, Error **errp);
>      void (*end_union)(Visitor *v, bool data_present, Error **errp);
>  };
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 88bed9c..be1fcdd 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -104,57 +104,48 @@ void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
>  {
>      uint64_t value;
>
> -    if (v->type_uint8) {
> -        v->type_uint8(v, obj, name, errp);
> -    } else {
> -        value = *obj;
> -        v->type_uint64(v, &value, name, errp);
> -        if (value > UINT8_MAX) {
> -            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> -                       name ? name : "null", "uint8_t");
> -            return;
> -        }
> -        *obj = value;
> +    value = *obj;
> +    v->type_uint64(v, &value, name, errp);
> +    if (value > UINT8_MAX) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> +                   name ? name : "null", "uint8_t");
> +        return;
>      }
> +    *obj = value;
>  }

Best reviewed with whitespace ignored.

Have you considered doing this before PATCH 02?  Hmm, swapping them now
is probably not worth the bother.

[...]
diff mbox

Patch

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 70326e0..d071c06 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -1,7 +1,7 @@ 
 /*
  * Core Definitions for QAPI Visitor implementations
  *
- * Copyright (C) 2012 Red Hat, Inc.
+ * Copyright (C) 2012, 2015 Red Hat, Inc.
  *
  * Author: Paolo Bonizni <pbonzini@redhat.com>
  *
@@ -36,6 +36,16 @@  struct Visitor
     void (*get_next_type)(Visitor *v, QType *type, bool promote_int,
                           const char *name, Error **errp);

+    /* Must be set. */
+    void (*type_int64)(Visitor *v, int64_t *obj, const char *name,
+                       Error **errp);
+    /* Must be set. */
+    void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name,
+                        Error **errp);
+    /* Only required to visit sizes differently than (*type_uint64)().  */
+    void (*type_size)(Visitor *v, uint64_t *obj, const char *name,
+                      Error **errp);
+    /* Must be set. */
     void (*type_bool)(Visitor *v, bool *obj, const char *name, Error **errp);
     void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
     void (*type_number)(Visitor *v, double *obj, const char *name,
@@ -46,16 +56,6 @@  struct Visitor
     /* May be NULL; most useful for input visitors. */
     void (*optional)(Visitor *v, bool *present, const char *name);

-    void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp);
-    void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
-    void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp);
-    void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
-    void (*type_int8)(Visitor *v, int8_t *obj, const char *name, Error **errp);
-    void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error **errp);
-    void (*type_int32)(Visitor *v, int32_t *obj, const char *name, Error **errp);
-    void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
-    /* visit_type_size() falls back to (*type_uint64)() if type_size is unset */
-    void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
     bool (*start_union)(Visitor *v, bool data_present, Error **errp);
     void (*end_union)(Visitor *v, bool data_present, Error **errp);
 };
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 88bed9c..be1fcdd 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -104,57 +104,48 @@  void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
 {
     uint64_t value;

-    if (v->type_uint8) {
-        v->type_uint8(v, obj, name, errp);
-    } else {
-        value = *obj;
-        v->type_uint64(v, &value, name, errp);
-        if (value > UINT8_MAX) {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                       name ? name : "null", "uint8_t");
-            return;
-        }
-        *obj = value;
+    value = *obj;
+    v->type_uint64(v, &value, name, errp);
+    if (value > UINT8_MAX) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   name ? name : "null", "uint8_t");
+        return;
     }
+    *obj = value;
 }

-void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp)
+void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name,
+                       Error **errp)
 {
     uint64_t value;

-    if (v->type_uint16) {
-        v->type_uint16(v, obj, name, errp);
-    } else {
-        value = *obj;
-        v->type_uint64(v, &value, name, errp);
-        if (value > UINT16_MAX) {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                       name ? name : "null", "uint16_t");
-            return;
-        }
-        *obj = value;
+    value = *obj;
+    v->type_uint64(v, &value, name, errp);
+    if (value > UINT16_MAX) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   name ? name : "null", "uint16_t");
+        return;
     }
+    *obj = value;
 }

-void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp)
+void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name,
+                       Error **errp)
 {
     uint64_t value;

-    if (v->type_uint32) {
-        v->type_uint32(v, obj, name, errp);
-    } else {
-        value = *obj;
-        v->type_uint64(v, &value, name, errp);
-        if (value > UINT32_MAX) {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                       name ? name : "null", "uint32_t");
-            return;
-        }
-        *obj = value;
+    value = *obj;
+    v->type_uint64(v, &value, name, errp);
+    if (value > UINT32_MAX) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   name ? name : "null", "uint32_t");
+        return;
     }
+    *obj = value;
 }

-void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
+void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name,
+                       Error **errp)
 {
     v->type_uint64(v, obj, name, errp);
 }
@@ -163,54 +154,42 @@  void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp)
 {
     int64_t value;

-    if (v->type_int8) {
-        v->type_int8(v, obj, name, errp);
-    } else {
-        value = *obj;
-        v->type_int64(v, &value, name, errp);
-        if (value < INT8_MIN || value > INT8_MAX) {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                       name ? name : "null", "int8_t");
-            return;
-        }
-        *obj = value;
+    value = *obj;
+    v->type_int64(v, &value, name, errp);
+    if (value < INT8_MIN || value > INT8_MAX) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   name ? name : "null", "int8_t");
+        return;
     }
+    *obj = value;
 }

 void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
 {
     int64_t value;

-    if (v->type_int16) {
-        v->type_int16(v, obj, name, errp);
-    } else {
-        value = *obj;
-        v->type_int64(v, &value, name, errp);
-        if (value < INT16_MIN || value > INT16_MAX) {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                       name ? name : "null", "int16_t");
-            return;
-        }
-        *obj = value;
+    value = *obj;
+    v->type_int64(v, &value, name, errp);
+    if (value < INT16_MIN || value > INT16_MAX) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   name ? name : "null", "int16_t");
+        return;
     }
+    *obj = value;
 }

 void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp)
 {
     int64_t value;

-    if (v->type_int32) {
-        v->type_int32(v, obj, name, errp);
-    } else {
-        value = *obj;
-        v->type_int64(v, &value, name, errp);
-        if (value < INT32_MIN || value > INT32_MAX) {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                       name ? name : "null", "int32_t");
-            return;
-        }
-        *obj = value;
+    value = *obj;
+    v->type_int64(v, &value, name, errp);
+    if (value < INT32_MIN || value > INT32_MAX) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   name ? name : "null", "int32_t");
+        return;
     }
+    *obj = value;
 }

 void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)