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<address@hidden>  wrote:
>
> > Base definitions/includes for Visiter interface used by generated
> > visiter/marshalling code.
> >
> > Signed-off-by: Michael Roth<address@hidden>
> > ---
> >   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<address@hidden>
> > + *
> > + * 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