Re: [Qemu-devel] [PATCH v8 35/35] RFC: qapi: Adjust layout of FooList types

[Marc-André Lureau]

Hi

On Mon, Dec 21, 2015 at 6:08 PM, Eric Blake <address@hidden> wrote:
> By sticking the next pointer first, we don't need a union with
> 64-bit padding for smaller types.  On 32-bit platforms, this
> can reduce the size of uint8List from 16 bytes (or 12, depending
> on whether 64-bit ints can tolerate 4-byte alignment) down to 8.
> It has no effect on 64-bit platforms (where alignment still
> dictates a 16-byte struct); but fewer anonymous unions is still
> a win in my book.

small win, but a win

> However, this requires visit_start_list() and visit_next_list()
> to gain a size parameter, to know what size element to allocate.

If this is the only "drawback", I'd be fine with it.

> I debated about going one step further, to allow for fewer casts,
> by doing:
>     typedef GenericList GenericList;
>     struct GenericList {
>         GenericList *next;
>     };
>     struct FooList {
>         GenericList base;
>         Foo value;
>     };
> so that you convert to 'GenericList *' by '&foolist->base', and
> back by 'container_of(generic, GenericList, base)' (as opposed to
> the existing '(GenericList *)foolist' and '(FooList *)generic').
> But doing that would require hoisting the declaration of
> GenericList prior to inclusion of qapi-types.h, rather than its
> current spot in visitor.h; it also makes iteration a bit more
> verbose through 'foolist->base.next' instead of 'foolist->next'.
>
> Signed-off-by: Eric Blake <address@hidden>
>

otherwise, patch looks good to me, waiting for non-rfc version if all
agree with it, for the review-by tag.

-- 
Marc-André Lureau