[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 1/5] qobject: ensure base is at offset 0
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH v6 1/5] qobject: ensure base is at offset 0 |
Date: |
Thu, 19 Apr 2018 17:23:02 +0200 |
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,
> with no change in behavior and where we are no longer dependent on using
> offset 0 (even though all current instances do). So, I'll leave that to
> maintainer discretion.
>
I don't think we have a good reason to allow offset different than 0.
The fact that we have code that rely on that behaviour already is a
sign that this could easily happen again, because it's the common
pattern in C for inheritance, and static casting is allowed, for
better or worse.
--
Marc-André Lureau
Re: [Qemu-devel] [PATCH v6 1/5] qobject: ensure base is at offset 0, Markus Armbruster, 2018/04/27
[Qemu-devel] [PATCH v6 2/5] qobject: use a QObjectBase_ struct, Marc-André Lureau, 2018/04/19
[Qemu-devel] [PATCH v6 5/5] qobject: modify qobject_ref() to assert on NULL, Marc-André Lureau, 2018/04/19