Re: [Qemu-devel] [PATCH v6 1/5] qobject: ensure base is at offset 0

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Marc-André Lureau <address@hidden> writes:
>
>> On Thu, Apr 19, 2018 at 5:20 PM, Eric Blake <address@hidden> wrote:
>>> On 04/19/2018 10:01 AM, Marc-André Lureau wrote:
>>>> All QObject types have the base QObject as their first field. This
>>>> allows the simplification of qobject_to().
>>>>
>>>> This explicitly guarantees that existing casts work correctly (even
>>>> though we'd prefer to get rid of such casts in any location except the
>>>> qobject.h macros)
>>>>
>>>> Signed-off-by: Marc-André Lureau <address@hidden>
>>>> Reviewed-by: Eric Blake <address@hidden>
>>>
>>> My R-b stands that this is correct from a coding point of view.  But if
>>> I read Markus' review correctly, we could omit this patch, fix the one
>>> broken client in tests/check-qdict.c to use qobject_to() (why didn't you
>>> fix that in v6)?, and then just apply patches 2-5 without this patch,
>
> Is that safe?

Yes, with a tweak to PATCH 2.

However, requiring base to come first is totally fine.  There's really
no reason to put it anywhere else, and fuzzing around with
container_of() is just complicating things.  Sunk cost (and thus not
worth changing) until this series, where keeping it would complicate the
next patch a bit, justifying this one.

I wouldn't have bothered with QEMU_BUILD_BUG(), let alone
QEMU_BUILD_BUG_MSG(); experience with QOM strongly indicates this
mistake is vanishingly unlikely in practice.  But I also can't be
bothered to rip it out.

[...]