Re: [Qemu-devel] [PATCH 0/8] qapi: add support for lists of native types

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

On Thu, May 09, 2013 at 03:31:08PM +0200, Laszlo Ersek wrote:
> On 05/09/13 01:33, Michael Roth wrote:
> > These patches apply on top of qemu.git master, and can also be obtained from:
> > git://github.com/mdroth/qemu.git qapi-native-lists
> > 
> > Sending this now since a number of series have popped up in the past that
> > wanted this, and Amos has some pending patches (query-mac-tables) that rely
> > on this as well.
> > 
> > These patches add support for specifying lists of native qapi types
> > (int/bool/str/number) like so:
> > 
> >   { 'type': 'foo',
> >     'data': { 'bar': ['int'] }}
> > 
> > for a 'bar' field that is a list of type 'int',
> > 
> >   { 'type': 'foo2',
> >     'data': { 'bar2': ['str'] }}
> > 
> > for a 'bar2' field that is a list of type 'str', and so on.
> > 
> > This uses linked list types for the native C representations, just as we do
> > for complex schema-defined types. In the future we may add schema annotations
> > of some sort to specify a more natural/efficient array type for the C
> > representations, but this should serve the majority of uses-cases for now.
> > 
> >  Makefile                           |    6 +-
> >  qapi-schema-test.json              |    8 ++
> >  scripts/qapi-types.py              |   44 ++++++-
> >  scripts/qapi-visit.py              |   36 ++++-
> >  scripts/qapi.py                    |   21 +++
> >  tests/test-qmp-input-visitor.c     |  181 +++++++++++++++++++++++++
> >  tests/test-qmp-output-visitor.c    |  172 ++++++++++++++++++++++++
> >  tests/test-visitor-serialization.c |  256 +++++++++++++++++++++++++++++++++---
> >  8 files changed, 692 insertions(+), 32 deletions(-)
> > 
> > 
> 
> Two notes:
> - the remark I made for 6/8 (comparing empty strings),
> 
> - for 7/8 and 8/8: the format specification %3.4f is not very useful.
> "3" is the field width, "4" is the precision, and the latter means for
> %f the number of digits printed after the radix character. It's not
> useful to specify a smaller field width (which covers the entire output
> string) than precision here.

Yup, that's a mistake. The field width isn't useful at all for what I
was intending actually (to constrain the whole portion of float so that
the fractional portion wouldn't get truncated by snprintf and make the
test less likely to catch re-encoding issues). I think just using %.4f
should suffice since we have plenty of extra buffer space for the values
we're working with (max being 32 / 3) so I'll do that for the next pass.

> 
> Other than these nothing pokes me in the eye.
> 
> Laszlo
>