Hi On Thu, Mar 29, 2018 at 6:10 PM, Eric Blake <ebl...@redhat.com> 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 <marcandre.lur...@redhat.com> >> --- > > >> @@ -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