[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v3 4/4] qobject: modify qobject_ref() to assert on N
From: |
Marc-André Lureau |
Subject: |
[Qemu-devel] [PATCH v3 4/4] qobject: modify qobject_ref() to assert on NULL and return obj |
Date: |
Thu, 29 Mar 2018 17:48:33 +0200 |
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>
---
include/qapi/qmp/qobject.h | 15 ++++++++++-----
block.c | 8 ++++----
block/blkdebug.c | 7 +++----
block/blkverify.c | 8 ++++----
block/null.c | 3 +--
block/nvme.c | 3 +--
block/quorum.c | 4 ++--
monitor.c | 19 +++++++------------
qapi/qobject-input-visitor.c | 6 ++----
qapi/qobject-output-visitor.c | 7 +++----
qobject/qdict.c | 33 +++++++++++----------------------
11 files changed, 48 insertions(+), 65 deletions(-)
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index 4183d81f00..bd3a618a73 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -70,11 +70,11 @@ static inline void qobject_init(QObject *obj, QType type)
obj->base.type = type;
}
-static inline void qobject_ref(QObject *obj)
+static inline void *qobject_ref(QObject *obj)
{
- if (obj) {
- obj->base.refcnt++;
- }
+ assert(obj);
+ obj->base.refcnt++;
+ return obj;
}
/**
@@ -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)
+ *
+ * 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))
/**
* qobject_unref(): Decrement QObject's reference count, deallocate
* when it reaches zero
+ * @obj: a #QObject or children type instance (can be NULL)
*/
#define qobject_unref(obj) \
qobject_unref(QOBJECT(obj))
diff --git a/block.c b/block.c
index 55a79845be..676e57f562 100644
--- a/block.c
+++ 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)));
found_any = true;
}
@@ -5207,8 +5207,8 @@ void bdrv_refresh_filename(BlockDriverState *bs)
* suffices without querying the (exact_)filename of this BDS. */
if (bs->file->bs->full_open_options) {
qdict_put_str(opts, "driver", drv->format_name);
- qobject_ref(bs->file->bs->full_open_options);
- qdict_put(opts, "file", bs->file->bs->full_open_options);
+ qdict_put(opts, "file",
+ qobject_ref(bs->file->bs->full_open_options));
bs->full_open_options = opts;
} else {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 689703d386..053372c22e 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -845,13 +845,12 @@ static void blkdebug_refresh_filename(BlockDriverState
*bs, QDict *options)
opts = qdict_new();
qdict_put_str(opts, "driver", "blkdebug");
- qobject_ref(bs->file->bs->full_open_options);
- qdict_put(opts, "image", bs->file->bs->full_open_options);
+ qdict_put(opts, "image", qobject_ref(bs->file->bs->full_open_options));
for (e = qdict_first(options); e; e = qdict_next(options, e)) {
if (strcmp(qdict_entry_key(e), "x-image")) {
- qobject_ref(qdict_entry_value(e));
- qdict_put_obj(opts, qdict_entry_key(e), qdict_entry_value(e));
+ qdict_put_obj(opts, qdict_entry_key(e),
+ qobject_ref(qdict_entry_value(e)));
}
}
diff --git a/block/blkverify.c b/block/blkverify.c
index 3cffcb1ca6..754cc9e857 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -291,10 +291,10 @@ static void blkverify_refresh_filename(BlockDriverState
*bs, QDict *options)
QDict *opts = qdict_new();
qdict_put_str(opts, "driver", "blkverify");
- qobject_ref(bs->file->bs->full_open_options);
- qdict_put(opts, "raw", bs->file->bs->full_open_options);
- qobject_ref(s->test_file->bs->full_open_options);
- qdict_put(opts, "test", s->test_file->bs->full_open_options);
+ qdict_put(opts, "raw",
+ qobject_ref(bs->file->bs->full_open_options));
+ qdict_put(opts, "test",
+ qobject_ref(s->test_file->bs->full_open_options));
bs->full_open_options = opts;
}
diff --git a/block/null.c b/block/null.c
index 700a2d0857..3944550f67 100644
--- a/block/null.c
+++ b/block/null.c
@@ -244,7 +244,6 @@ static int coroutine_fn
null_co_block_status(BlockDriverState *bs,
static void null_refresh_filename(BlockDriverState *bs, QDict *opts)
{
- qobject_ref(opts);
qdict_del(opts, "filename");
if (!qdict_size(opts)) {
@@ -253,7 +252,7 @@ static void null_refresh_filename(BlockDriverState *bs,
QDict *opts)
}
qdict_put_str(opts, "driver", bs->drv->format_name);
- bs->full_open_options = opts;
+ bs->full_open_options = qobject_ref(opts);
}
static BlockDriver bdrv_null_co = {
diff --git a/block/nvme.c b/block/nvme.c
index e192da9ee1..6f71122bf5 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1073,7 +1073,6 @@ static int nvme_reopen_prepare(BDRVReopenState
*reopen_state,
static void nvme_refresh_filename(BlockDriverState *bs, QDict *opts)
{
- qobject_ref(opts);
qdict_del(opts, "filename");
if (!qdict_size(opts)) {
@@ -1082,7 +1081,7 @@ static void nvme_refresh_filename(BlockDriverState *bs,
QDict *opts)
}
qdict_put_str(opts, "driver", bs->drv->format_name);
- bs->full_open_options = opts;
+ bs->full_open_options = qobject_ref(opts);
}
static void nvme_refresh_limits(BlockDriverState *bs, Error **errp)
diff --git a/block/quorum.c b/block/quorum.c
index 862cea366d..a5051da56e 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1082,8 +1082,8 @@ static void quorum_refresh_filename(BlockDriverState *bs,
QDict *options)
children = qlist_new();
for (i = 0; i < s->num_children; i++) {
- qobject_ref(s->children[i]->bs->full_open_options);
- qlist_append(children, s->children[i]->bs->full_open_options);
+ qlist_append(children,
+ qobject_ref(s->children[i]->bs->full_open_options));
}
opts = qdict_new();
diff --git a/monitor.c b/monitor.c
index 9d2b89431e..c149cf9e96 100644
--- a/monitor.c
+++ b/monitor.c
@@ -494,9 +494,8 @@ static void monitor_json_emitter(Monitor *mon, QObject
*data)
* caller won't free the data (which will be finally freed in
* responder thread).
*/
- qobject_ref(data);
qemu_mutex_lock(&mon->qmp.qmp_queue_lock);
- g_queue_push_tail(mon->qmp.qmp_responses, data);
+ g_queue_push_tail(mon->qmp.qmp_responses, qobject_ref(data));
qemu_mutex_unlock(&mon->qmp.qmp_queue_lock);
qemu_bh_schedule(mon_global.qmp_respond_bh);
} else {
@@ -614,8 +613,7 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict,
Error **errp)
* replacing a prior stored event if any.
*/
qobject_unref(evstate->qdict);
- evstate->qdict = qdict;
- qobject_ref(evstate->qdict);
+ evstate->qdict = qobject_ref(qdict);
} else {
/*
* Last send was (at least) evconf->rate ns ago.
@@ -629,8 +627,7 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict,
Error **errp)
evstate = g_new(MonitorQAPIEventState, 1);
evstate->event = event;
- evstate->data = data;
- qobject_ref(evstate->data);
+ evstate->data = qobject_ref(data);
evstate->qdict = NULL;
evstate->timer = timer_new_ns(event_clock_type,
monitor_qapi_event_handler,
@@ -4049,8 +4046,7 @@ static void monitor_qmp_respond(Monitor *mon, QObject
*rsp,
if (rsp) {
if (id) {
/* This is for the qdict below. */
- qobject_ref(id);
- qdict_put_obj(qobject_to(QDict, rsp), "id", id);
+ qdict_put_obj(qobject_to(QDict, rsp), "id", qobject_ref(id));
}
monitor_json_emitter(mon, rsp);
@@ -4190,15 +4186,14 @@ static void handle_qmp_command(JSONMessageParser
*parser, GQueue *tokens)
goto err;
}
- qobject_ref(id);
- qdict_del(qdict, "id");
-
req_obj = g_new0(QMPRequest, 1);
req_obj->mon = mon;
- req_obj->id = id;
+ req_obj->id = id ? qobject_ref(id) : NULL;
req_obj->req = req;
req_obj->need_resume = false;
+ qdict_del(qdict, "id");
+
if (qmp_is_oob(qdict)) {
/* Out-Of-Band (OOB) requests are executed directly in parser. */
trace_monitor_qmp_cmd_out_of_band(qobject_get_try_str(req_obj->id)
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 7a290c4a3f..da57f4cc24 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -588,8 +588,7 @@ static void qobject_input_type_any(Visitor *v, const char
*name, QObject **obj,
return;
}
- qobject_ref(qobj);
- *obj = qobj;
+ *obj = qobject_ref(qobj);
}
static void qobject_input_type_null(Visitor *v, const char *name,
@@ -677,8 +676,7 @@ static QObjectInputVisitor
*qobject_input_visitor_base_new(QObject *obj)
v->visitor.optional = qobject_input_optional;
v->visitor.free = qobject_input_free;
- v->root = obj;
- qobject_ref(obj);
+ v->root = qobject_ref(obj);
return v;
}
diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
index 3a933b489b..89ffd8a7bf 100644
--- a/qapi/qobject-output-visitor.c
+++ b/qapi/qobject-output-visitor.c
@@ -188,8 +188,8 @@ static void qobject_output_type_any(Visitor *v, const char
*name,
QObject **obj, Error **errp)
{
QObjectOutputVisitor *qov = to_qov(v);
- qobject_ref(*obj);
- qobject_output_add_obj(qov, name, *obj);
+
+ qobject_output_add_obj(qov, name, qobject_ref(*obj));
}
static void qobject_output_type_null(Visitor *v, const char *name,
@@ -210,8 +210,7 @@ static void qobject_output_complete(Visitor *v, void
*opaque)
assert(qov->root && QSLIST_EMPTY(&qov->stack));
assert(opaque == qov->result);
- qobject_ref(qov->root);
- *qov->result = qov->root;
+ *qov->result = qobject_ref(qov->root);
qov->result = NULL;
}
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 2e9bd53e22..22800eeceb 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -373,8 +373,7 @@ QDict *qdict_clone_shallow(const QDict *src)
for (i = 0; i < QDICT_BUCKET_MAX; i++) {
QLIST_FOREACH(entry, &src->table[i], next) {
- qobject_ref(entry->value);
- qdict_put_obj(dest, entry->key, entry->value);
+ qdict_put_obj(dest, entry->key, qobject_ref(entry->value));
}
}
@@ -480,8 +479,7 @@ void qdict_copy_default(QDict *dst, QDict *src, const char
*key)
val = qdict_get(src, key);
if (val) {
- qobject_ref(val);
- qdict_put_obj(dst, key, val);
+ qdict_put_obj(dst, key, qobject_ref(val));
}
}
@@ -526,8 +524,7 @@ static void qdict_flatten_qlist(QList *qlist, QDict
*target, const char *prefix)
qdict_flatten_qlist(qobject_to(QList, value), target, new_key);
} else {
/* All other types are moved to the target unchanged. */
- qobject_ref(value);
- qdict_put_obj(target, new_key, value);
+ qdict_put_obj(target, new_key, qobject_ref(value));
}
g_free(new_key);
@@ -566,8 +563,7 @@ static void qdict_flatten_qdict(QDict *qdict, QDict
*target, const char *prefix)
delete = true;
} else if (prefix) {
/* All other objects are moved to the target unchanged. */
- qobject_ref(value);
- qdict_put_obj(target, new_key, value);
+ qdict_put_obj(target, new_key, qobject_ref(value));
delete = true;
}
@@ -610,8 +606,7 @@ void qdict_extract_subqdict(QDict *src, QDict **dst, const
char *start)
while (entry != NULL) {
next = qdict_next(src, entry);
if (strstart(entry->key, start, &p)) {
- qobject_ref(entry->value);
- qdict_put_obj(*dst, p, entry->value);
+ qdict_put_obj(*dst, p, qobject_ref(entry->value));
qdict_del(src, entry->key);
}
entry = next;
@@ -894,16 +889,14 @@ QObject *qdict_crumple(const QDict *src, Error **errp)
qdict_put_obj(two_level, prefix, QOBJECT(child_dict));
}
- qobject_ref(ent->value);
- qdict_put_obj(child_dict, suffix, ent->value);
+ qdict_put_obj(child_dict, suffix, qobject_ref(ent->value));
} else {
if (child) {
error_setg(errp, "Key %s prefix is already set as a dict",
prefix);
goto error;
}
- qobject_ref(ent->value);
- qdict_put_obj(two_level, prefix, ent->value);
+ qdict_put_obj(two_level, prefix, qobject_ref(ent->value));
}
g_free(prefix);
@@ -924,8 +917,7 @@ QObject *qdict_crumple(const QDict *src, Error **errp)
qdict_put_obj(multi_level, ent->key, child);
} else {
- qobject_ref(ent->value);
- qdict_put_obj(multi_level, ent->key, ent->value);
+ qdict_put_obj(multi_level, ent->key, qobject_ref(ent->value));
}
}
qobject_unref(two_level);
@@ -951,8 +943,7 @@ QObject *qdict_crumple(const QDict *src, Error **errp)
goto error;
}
- qobject_ref(child);
- qlist_append_obj(qobject_to(QList, dst), child);
+ qlist_append_obj(qobject_to(QList, dst), qobject_ref(child));
}
qobject_unref(multi_level);
multi_level = NULL;
@@ -1055,8 +1046,7 @@ void qdict_join(QDict *dest, QDict *src, bool overwrite)
next = qdict_next(src, entry);
if (overwrite || !qdict_haskey(dest, entry->key)) {
- qobject_ref(entry->value);
- qdict_put_obj(dest, entry->key, entry->value);
+ qdict_put_obj(dest, entry->key, qobject_ref(entry->value));
qdict_del(src, entry->key);
}
@@ -1088,8 +1078,7 @@ bool qdict_rename_keys(QDict *qdict, const QDictRenames
*renames, Error **errp)
}
qobj = qdict_get(qdict, renames->from);
- qobject_ref(qobj);
- qdict_put_obj(qdict, renames->to, qobj);
+ qdict_put_obj(qdict, renames->to, qobject_ref(qobj));
qdict_del(qdict, renames->from);
}
--
2.17.0.rc1.36.gcedb63ea2f