Re: [Qemu-devel] [PATCH 1/8] qapi: Add Visitor interfaces for uint*_t and int*_t

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On Sat, Jun 09, 2012 at 05:33:14PM +0200, Laszlo Ersek wrote:
> On 06/09/12 17:16, Andreas Färber wrote:
> > Am 09.06.2012 17:03, schrieb Laszlo Ersek:
> >> On 06/08/12 17:35, Andreas Färber wrote:
> >>> From: Michael Roth <mdroth@linux.vnet.ibm.com>
> >>>
> >>> This adds visitor interfaces for fixed-width integers types.
> >>> Implementing these in visitors is optional, otherwise we fall back to
> >>> visit_type_int() (int64_t) with some additional bounds checking to avoid
> >>> integer overflows for cases where the value fetched exceeds the bounds
> >>> of our target C type.
> >>
> >>> diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
> >>> index e850746..a19d70c 100644
> >>> --- a/qapi/qapi-visit-core.h
> >>> +++ b/qapi/qapi-visit-core.h
> >>> @@ -52,6 +52,14 @@ struct Visitor
> >>>      void (*start_handle)(Visitor *v, void **obj, const char *kind,
> >>>                           const char *name, Error **errp);
> >>>      void (*end_handle)(Visitor *v, Error **errp);
> >>> +    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);
> >>>  };
> >>>  
> >>>  void visit_start_handle(Visitor *v, void **obj, const char *kind,
> >>> @@ -69,6 +77,14 @@ void visit_end_optional(Visitor *v, Error **errp);
> >>>  void visit_type_enum(Visitor *v, int *obj, const char *strings[],
> >>>                       const char *kind, const char *name, Error **errp);
> >>>  void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp);
> >>> +void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp);
> >>> +void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp);
> >>> +void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp);
> >>> +void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp);
> >>> +void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp);
> >>> +void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp);
> >>> +void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp);
> >>> +void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
> >>>  void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
> >>>  void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
> >>>  void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
> >>
> >> Shouldn't "scripts/qapi.py" be extended accordingly? (The c_type function.)
> > 
> > What does that affect?
> > Is it a blocker for this PULL or an improvement for a follow-up?
> 
> The latter. As long as nothing actually uses these types there's no problem.

To clarify: as long as nobody uses these types in a QAPI schema it's
fine.

The current users don't rely on schema-generated fixed-width types so
we're okay as far as the pull.

There's an extension to c_type() to support fixed-width types floating around
in the QIDL series, but a standalone patch at some point would be warranted
as well.

> 
> Laszlo
>