[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 00/36] drop qapi nested structs
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v6 00/36] drop qapi nested structs |
Date: |
Tue, 28 Apr 2015 16:02:46 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> writes:
> We want to eventually allow qapi defaults, by making:
> 'data':{'*flag':'bool'}
> as shorthand for something like:
> 'data':{'flag':{'type':'bool', 'optional':true}}
> so that the default can be specified:
> 'data':{'flag':{'type':'bool', 'optional':true, 'default':true}}
>
> This series does not quite get us there, but it DOES do a number
> of other things. It gets rid of the three uses of nested inline
> structs, changes anonymous unions to use a specific 'alternate'
> metatype rather than abusing 'union', changes the ambiguous 'type'
> to the obvious 'struct', and fixes lots of other parser bugs found
> while designing the testsuite. The testsuite changes make the
> bulk of this series, with a repeating pattern of writing tests
> that expose weaknesses in the old parser, then beefing up the
> generator to catch the problem during the initial parse rather
> than choking with an obscure python message or even causing a C
> compilation failure. There are a couple of ideas for things to
> still fix before all is said and done (such as preventing a
> collision of members between a struct and its base class, or
> fixing the parser to understand \uXXXX escape sequences in
> strings), but at this point, anything raised as a review comment
> is probably better addressed as followup patches rather than
> respinning a v7.
>
> v5 was here:
> https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg05034.html
>
> v6 changes are noted in each patch; in particular, several new
> patches were added (additional tests, split some patches, conversion
> to 'struct' instead of 'type'). But most of the changes were in
> direct response to review comments or rebase fallout, so I kept
> in Reviewed-by markings where possible, to help focus review on
> the remainder.
Looks good to go to me, except for new PATCH 37, where I suggested a
small change. Shouldn't hold up this series. If any other patch still
lacks my R-by, let me know.
> I wrote another patch while working on this series, but it was
> independent enough that I posted it separately (although I based
> the documentation in this patch as if that, or Markus' alternative,
> had been applied):
> https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg00373.html
Both need a trivial respin to correct a pasto. Yours has a more
elaborate commit message, and a test. Mine is less code, in part
because it uses a single qnull object instead of allocating one for each
use, and it has separate patches for the qobject and the json-parser
change.
We can pick one, of we can combine the best of both into a new
mini-series. Preferences?
- Re: [Qemu-devel] [PATCH v6 32/36] qapi: Drop tests for inline nested structs, (continued)