Marc-André Lureau <marcandre.lur...@redhat.com> writes:
> Add a new function qdict_steal(), that deletes a key from a dict, and
> returns the associated value, if any. Simplify related code.
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> include/qapi/qmp/qdict.h | 1 +
> monitor.c | 3 +--
> qobject/block-qdict.c | 7 ++-----
> qobject/qdict.c | 22 ++++++++++++++++++++++
> 4 files changed, 26 insertions(+), 7 deletions(-)
Missing: tests/check-qdict.c update.
> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
> index 7f3ec10a10..84644bfba6 100644
> --- a/include/qapi/qmp/qdict.h
> +++ b/include/qapi/qmp/qdict.h
> @@ -37,6 +37,7 @@ QObject *qdict_entry_value(const QDictEntry *entry);
> size_t qdict_size(const QDict *qdict);
> void qdict_put_obj(QDict *qdict, const char *key, QObject *value);
> void qdict_del(QDict *qdict, const char *key);
> +QObject *qdict_steal(QDict *qdict, const char *key);
> int qdict_haskey(const QDict *qdict, const char *key);
> QObject *qdict_get(const QDict *qdict, const char *key);
> bool qdict_is_equal(const QObject *x, const QObject *y);
> diff --git a/monitor.c b/monitor.c
> index a1999e396c..eab2dc7b7b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4262,8 +4262,7 @@ static void handle_qmp_command(JSONMessageParser
> *parser, GQueue *tokens)
>
> qdict = qobject_to(QDict, req);
> if (qdict) {
> - id = qobject_ref(qdict_get(qdict, "id"));
> - qdict_del(qdict, "id");
> + id = qdict_steal(qdict, "id");
> } /* else will fail qmp_dispatch() */
>
> if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
> diff --git a/qobject/block-qdict.c b/qobject/block-qdict.c
> index 42054cc274..a07664ee6a 100644
> --- a/qobject/block-qdict.c
> +++ b/qobject/block-qdict.c
> @@ -692,8 +692,6 @@ void qdict_join(QDict *dest, QDict *src, bool overwrite)
> */
> bool qdict_rename_keys(QDict *qdict, const QDictRenames *renames, Error
> **errp)
> {
> - QObject *qobj;
> -
> while (renames->from) {
> if (qdict_haskey(qdict, renames->from)) {
> if (qdict_haskey(qdict, renames->to)) {
> @@ -702,9 +700,8 @@ bool qdict_rename_keys(QDict *qdict, const QDictRenames
> *renames, Error **errp)
> return false;
> }
>
> - qobj = qdict_get(qdict, renames->from);
> - qdict_put_obj(qdict, renames->to, qobject_ref(qobj));
> - qdict_del(qdict, renames->from);
> + qdict_put_obj(qdict, renames->to,
> + qdict_steal(qdict, renames->from));
> }
>
> renames++;
How did you find possible uses for qdict_steal()?
> diff --git a/qobject/qdict.c b/qobject/qdict.c
> index 3d8c2f7bbc..322f23f094 100644
> --- a/qobject/qdict.c
> +++ b/qobject/qdict.c
> @@ -406,6 +406,28 @@ void qdict_del(QDict *qdict, const char *key)
> }
> }
>
> +/**
> + * qdict_steal(): Steal a 'key:value' pair from the dictionary
"Steal" is cute, but perhaps qdict_get_and_del() would be clearer.
> + *
> + * Return the associated entry value and remove it, or NULL.
Suggest
* If @qdict has @key, return its value, and delete it from @qdict.
* Else, return NULL.
> + */
> +QObject *qdict_steal(QDict *qdict, const char *key)
> +{
> + QDictEntry *entry;
> + QObject *val = NULL;
> +
> + entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_BUCKET_MAX);
> + if (entry) {
> + QLIST_REMOVE(entry, next);
> + val = qobject_ref(entry->value);
> + qentry_destroy(entry);
> + qdict->size--;
> + }
> +
> + return val;
> +}
This is a slightly extended qdict_del().
void qdict_del(QDict *qdict, const char *key)
{
QDictEntry *entry;
entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_BUCKET_MAX);
if (entry) {
QLIST_REMOVE(entry, next);
qentry_destroy(entry);
qdict->size--;
}
}
Heavy-handed de-duplication:
QObject *qdict_get_and_del(QDict *qdict, const char *key)
{
QDictEntry *entry;
QObject *val;
entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_BUCKET_MAX);
if (!entry) {
return NULL;
}
QLIST_REMOVE(entry, next);
val = qobject_ref(entry->value);
qentry_destroy(entry);
qdict->size--;
return val;
}
void qdict_del(QDict *qdict, const char *key)
{
qobject_unref(qdict_get_and_del(qdict, key));
}
Makes qdict_del() execute an additional qobject_ref() / qobject_unref(),
but that shouldn't matter.
> +
> +
Just one blank line, please.
> /**
> * qdict_is_equal(): Test whether the two QDicts are equal
> *
