[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 4/4] qobject: modify qobject_ref() to assert
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH v3 4/4] qobject: modify qobject_ref() to assert on NULL and return obj |
Date: |
Fri, 13 Apr 2018 18:05:42 +0200 |
Hi
On Thu, Mar 29, 2018 at 6:10 PM, Eric Blake <address@hidden> wrote:
> On 03/29/2018 10:48 AM, Marc-André Lureau wrote:
>>
>> Following a discussion on the mailing list: while it may be convenient
>> to accept NULL value in qobject_unref() (for similar reasons as free()
>> accepts NULL), it is a probably a bad idea to accept NULL argument in
>> qobject_ref().
>>
>> Furthermore, it is convenient and more clear to call qobject_ref() at
>> the time when the reference is associated with a variable, or
>> argument. For this reason, make qobject_ref() return the same pointer
>> as given.
>>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>> ---
>
>
>> @@ -101,13 +101,18 @@ static inline void qobject_unref(QObject *obj)
>> /**
>> * qobject_ref(): Increment QObject's reference count
>> + * @obj: a #QObject or children type instance (must not be NULL)
>
>
> s/children/child/
>
>> + *
>> + * Returns: the same @obj. The type of @obj will be propagated to the
>> + * return type.
>> */
>> #define qobject_ref(obj) \
>> - qobject_ref(QOBJECT(obj))
>> + (typeof(obj)) qobject_ref(QOBJECT(obj))
>
>
> You're missing outer (). There are cases where '(cast) (expr)' and
> '((cast)(expr))' can behave differently when combined with surrounding
> syntax; for example, -> has higher precedence than cast. Consider:
>
> qobject_ref(my_qdict)->size;
>
> As you wrote it, you would attempt to dereference the 'size' member of void*
> (compile error), prior to casting that result to QDict*; with the
> parenthesis, you get the intended dereference of the size member of the
> QDict pointer.
>
ok
>> +++ b/block.c
>> @@ -5134,8 +5134,8 @@ static bool append_open_options(QDict *d,
>> BlockDriverState *bs)
>> continue;
>> }
>> - qobject_ref(qdict_entry_value(entry));
>> - qdict_put_obj(d, qdict_entry_key(entry),
>> qdict_entry_value(entry));
>> + qdict_put_obj(d, qdict_entry_key(entry),
>> + qobject_ref(qdict_entry_value(entry)));
>
>
> However, I like this simplification that your patch enables. How did you
> find candidate spots? Manually, or via Coccinelle?
Manual review (there are not that many qobject_ref())
thanks
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v3 4/4] qobject: modify qobject_ref() to assert on NULL and return obj,
Marc-André Lureau <=