[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: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v3 4/4] qobject: modify qobject_ref() to assert on NULL and return obj |
Date: |
Thu, 29 Mar 2018 11:10:47 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
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.
+++ 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?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
- [Qemu-devel] [PATCH v3 0/4] RFC: simplify qobject refcount, Marc-André Lureau, 2018/03/29
- [Qemu-devel] [PATCH v3 0/4] RFC: simplify qobject refcount, Marc-André Lureau, 2018/03/29
- [Qemu-devel] [PATCH v3 1/4] qobject: ensure base is at offset 0, Marc-André Lureau, 2018/03/29
- [Qemu-devel] [PATCH v3 2/4] qobject: introduce QObjectCommon, Marc-André Lureau, 2018/03/29
- [Qemu-devel] [PATCH v3 4/4] qobject: modify qobject_ref() to assert on NULL and return obj, Marc-André Lureau, 2018/03/29
- Re: [Qemu-devel] [PATCH v3 4/4] qobject: modify qobject_ref() to assert on NULL and return obj,
Eric Blake <=
- [Qemu-devel] [PATCH v3 3/4] qobject: replace qobject_incref/QINCREF qobject_decref/QDECREF, Marc-André Lureau, 2018/03/29
- Re: [Qemu-devel] [PATCH v3 0/4] RFC: simplify qobject refcount, no-reply, 2018/03/31