Re: [Qemu-devel] [PATCH v2][ 09/21] qapi: add qapi-visit-core.h

[Anthony Liguori]

On 06/09/2011 10:14 AM, Luiz Capitulino wrote:
On Fri,  3 Jun 2011 17:33:07 -0500
Michael Roth<mdr...@linux.vnet.ibm.com>  wrote:
Base definitions/includes for Visiter interface used by generated
visiter/marshalling code.

Signed-off-by: Michael Roth<mdr...@linux.vnet.ibm.com>
---
  qapi/qapi-visit-core.h |  187 ++++++++++++++++++++++++++++++++++++++++++++++++
  1 files changed, 187 insertions(+), 0 deletions(-)
  create mode 100644 qapi/qapi-visit-core.h

diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
new file mode 100644
index 0000000..a5ba016
--- /dev/null
+++ b/qapi/qapi-visit-core.h
@@ -0,0 +1,187 @@
+/*
+ * Core Definitions for QAPI Visiter Classes
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori<aligu...@us.ibm.com>
+ *
+ * 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.
+ *
+ */
+#ifndef QAPI_VISITER_CORE_H
+#define QAPI_VISITER_CORE_H
+
+#include "qapi/qapi-types-core.h"
+#include "error.h"
+#include<stdlib.h>
+
+typedef struct GenericList
+{
+    void *value;
+    struct GenericList *next;
+} GenericList;

If there's a reason to not use our list API, it should be explained in
the commit log.

Our lists require an embedded element.  Since these types are generated, 
if you want to use them in a different type of data structure, there's 
no easy way to add another embedded element.

The solution is to have non-embedded lists and that what this is.

Regards,

Anthony Liguori


+
+typedef struct Visiter Visiter;
+
+struct Visiter
+{
+    /* Must be set */
+    void (*start_struct)(Visiter *v, void **obj, const char *kind, const char 
*name, Error **errp);
+    void (*end_struct)(Visiter *v, Error **errp);
+
+    void (*start_list)(Visiter *v, const char *name, Error **errp);
+    GenericList *(*next_list)(Visiter *v, GenericList **list, Error **errp);
+    void (*end_list)(Visiter *v, Error **errp);
+
+    void (*type_enum)(Visiter *v, int *obj, const char *kind, const char 
*name, Error **errp);
+
+    void (*type_int)(Visiter *v, int64_t *obj, const char *name, Error **errp);
+    void (*type_bool)(Visiter *v, bool *obj, const char *name, Error **errp);
+    void (*type_str)(Visiter *v, char **obj, const char *name, Error **errp);
+    void (*type_number)(Visiter *v, double *obj, const char *name, Error 
**errp);
+
+    /* May be NULL */
+    void (*start_optional)(Visiter *v, bool *present, const char *name, Error 
**errp);
+    void (*end_optional)(Visiter *v, Error **errp);
+
+    void (*start_handle)(Visiter *v, void **obj, const char *kind, const char 
*name, Error **errp);
+    void (*end_handle)(Visiter *v, Error **errp);
+};
+
+static inline void visit_start_handle(Visiter *v, void **obj, const char 
*kind, const char *name, Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (v->start_handle) {
+        v->start_handle(v, obj, kind, name, errp);
+    }
+}

IMO, all these inlines should be real functions and...

+
+static inline void visit_end_handle(Visiter *v, Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (v->end_handle) {
+        v->end_handle(v, errp);
+    }
+}
+
+static inline void visit_start_struct(Visiter *v, void **obj, const char 
*kind, const char *name, Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    v->start_struct(v, obj, kind, name, errp);
+}

... this could be:

if (!error_is_set(errp)) {
     v->start_struct(v, obj, kind, name, errp);
}

Also valid for other functions in this file.

+
+static inline void visit_end_struct(Visiter *v, Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    v->end_struct(v, errp);
+}
+
+static inline void visit_start_list(Visiter *v, const char *name, Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    v->start_list(v, name, errp);
+}
+
+static inline GenericList *visit_next_list(Visiter *v, GenericList **list, 
Error **errp)
+{
+    if (error_is_set(errp)) {
+        return 0;
+    }
+
+    return v->next_list(v, list, errp);
+}
+
+static inline void visit_end_list(Visiter *v, Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    v->end_list(v, errp);
+}
+
+static inline void visit_start_optional(Visiter *v, bool *present, const char 
*name, Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (v->start_optional) {
+        v->start_optional(v, present, name, errp);
+    }
+}
+
+static inline void visit_end_optional(Visiter *v, Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (v->end_optional) {
+        v->end_optional(v, errp);
+    }
+}
+
+static inline void visit_type_enum(Visiter *v, int *obj, const char *kind, 
const char *name, Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    v->type_enum(v, obj, kind, name, errp);
+}
+
+static inline void visit_type_int(Visiter *v, int64_t *obj, const char *name, 
Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    v->type_int(v, obj, name, errp);
+}
+
+static inline void visit_type_bool(Visiter *v, bool *obj, const char *name, 
Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    v->type_bool(v, obj, name, errp);
+}
+
+static inline void visit_type_str(Visiter *v, char **obj, const char *name, 
Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    v->type_str(v, obj, name, errp);
+}
+
+static inline void visit_type_number(Visiter *v, double *obj, const char 
*name, Error **errp)
+{
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    v->type_number(v, obj, name, errp);
+}
+
+#endif