Re: [Qemu-devel] [PATCH v5 0/5] Simplify qobject refcount

[Marc-André Lureau]

Hi

On Thu, Apr 19, 2018 at 8:44 AM, Markus Armbruster <address@hidden> wrote:
> Marc-André Lureau <address@hidden> writes:
>
>> Hi,
>>
>> This series aims to get rid of the distinction between QObject, that
>> must use qobject_incref/qobject_decref and its various derived types
>> that have to use QINCREF/QDECREF. Instead, replace it with
>> qobject_ref/qobject_unref for all types.
>
> This is a lovely improvement.

That's a good sign.

> However, I think the code can't quite decide whether to embrace or
> eschew type casts between QObject and its subtypes.

It uses both, for different reasons

> On the one hand, we provide safe conversion macros QOBJECT() and
> qobject_to().  By the way, shouting one but not the other is a bit ugly.

QOBJECT is static upcast, the compiler will shout.
qobject_to() is dynamic downcast, the runtime shout.

> On the other hand, we still use type casts, and enforce "base comes
> first" to make them work.
>
> If we want to eschew type casts, we should clean up the remaining ones.
> The case for "base must come first" is quite weak then.  Not worth the
> bother, I think.
>
> I doubt we want to embrace type casts here.  If we wanted to, I'd go all
> the way and unwrap base in QObjects and its subtypes with suitable "must
> match QObject" comments.  Alternatively, use an unnamed member.  Then
> convert with type casts rather than container_of() and &->base.
>

Can you make a different patch? Can it be done after? Should we add a
note there for future improvements?

I don't like to spent too much time on this, yet as you said, this is
a lovely improvement, so is there any real blocker left?

-- 
Marc-André Lureau