[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 36/37] RFC: qapi: Adjust layout of FooList ty
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v9 36/37] RFC: qapi: Adjust layout of FooList types |
Date: |
Thu, 28 Jan 2016 10:23:27 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
On 01/28/2016 08:34 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> 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.
>>
>> However, this requires visit_start_list() and visit_next_list()
>> to gain a size parameter, to know what size element to allocate.
>>
>> 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'.
Should I attempt this?
>> typedef struct GenericList
>> {
>> - union {
>> - void *value;
>> - uint64_t padding;
>> - };
>> struct GenericList *next;
>> + char padding[];
>> } GenericList;
>
> Less trickery, I like it.
>
> Member padding appears to be unused.
or just leave it at this?
>> bool visit_start_list(Visitor *v, const char *name, GenericList **list,
>> - Error **errp)
>> + size_t size, Error **errp)
>> {
>> - bool result = v->start_list(v, name, list, errp);
>> + bool result;
>> +
>> + assert(list ? size : !size);
>
> Tighter than size != 0 would be size >= GenericList. Same elsewhere.
Makes sense.
>
> Rest looks good. Didn't look as closely as for the previous patches
> (getting tired), but so far I like the idea.
Okay, I'll keep it and drop the RFC.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v9 35/37] qapi: Change visit_type_FOO() to no longer return partial objects, Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 34/37] qapi: Simplify semantics of visit_next_list(), Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 15/37] qom: Swap 'name' next to visitor in ObjectPropertyAccessor, Eric Blake, 2016/01/19