* [PATCH for-5.1 1/5] qobject: Clean up QLIST_FOREACH_ENTRY()
2020-04-15 8:30 [PATCH for-5.1 0/5] qobject: Minor spring cleaning Markus Armbruster
@ 2020-04-15 8:30 ` Markus Armbruster
2020-04-15 12:25 ` Eric Blake
2020-04-15 8:30 ` [PATCH for-5.1 2/5] qobject: Factor out helper json_pretty_newline() Markus Armbruster
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2020-04-15 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth
QLIST_FOREACH_ENTRY() traverses a tail queue manually. Use
QTAILQ_FIRST() and QTAILQ_NEXT() instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/qapi/qmp/qlist.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 8d2c32ca28..07ecae81e4 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -34,10 +34,10 @@ void qlist_append_int(QList *qlist, int64_t value);
void qlist_append_null(QList *qlist);
void qlist_append_str(QList *qlist, const char *value);
-#define QLIST_FOREACH_ENTRY(qlist, var) \
- for ((var) = ((qlist)->head.tqh_first); \
- (var); \
- (var) = ((var)->next.tqe_next))
+#define QLIST_FOREACH_ENTRY(qlist, var) \
+ for ((var) = QTAILQ_FIRST(&(qlist)->head); \
+ (var); \
+ (var) = QTAILQ_NEXT((var), next))
static inline QObject *qlist_entry_obj(const QListEntry *entry)
{
--
2.21.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH for-5.1 1/5] qobject: Clean up QLIST_FOREACH_ENTRY()
2020-04-15 8:30 ` [PATCH for-5.1 1/5] qobject: Clean up QLIST_FOREACH_ENTRY() Markus Armbruster
@ 2020-04-15 12:25 ` Eric Blake
0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2020-04-15 12:25 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mdroth
On 4/15/20 3:30 AM, Markus Armbruster wrote:
> QLIST_FOREACH_ENTRY() traverses a tail queue manually. Use
> QTAILQ_FIRST() and QTAILQ_NEXT() instead.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> include/qapi/qmp/qlist.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
> diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
> index 8d2c32ca28..07ecae81e4 100644
> --- a/include/qapi/qmp/qlist.h
> +++ b/include/qapi/qmp/qlist.h
> @@ -34,10 +34,10 @@ void qlist_append_int(QList *qlist, int64_t value);
> void qlist_append_null(QList *qlist);
> void qlist_append_str(QList *qlist, const char *value);
>
> -#define QLIST_FOREACH_ENTRY(qlist, var) \
> - for ((var) = ((qlist)->head.tqh_first); \
> - (var); \
> - (var) = ((var)->next.tqe_next))
> +#define QLIST_FOREACH_ENTRY(qlist, var) \
> + for ((var) = QTAILQ_FIRST(&(qlist)->head); \
> + (var); \
> + (var) = QTAILQ_NEXT((var), next))
>
> static inline QObject *qlist_entry_obj(const QListEntry *entry)
> {
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH for-5.1 2/5] qobject: Factor out helper json_pretty_newline()
2020-04-15 8:30 [PATCH for-5.1 0/5] qobject: Minor spring cleaning Markus Armbruster
2020-04-15 8:30 ` [PATCH for-5.1 1/5] qobject: Clean up QLIST_FOREACH_ENTRY() Markus Armbruster
@ 2020-04-15 8:30 ` Markus Armbruster
2020-04-15 12:28 ` Eric Blake
2020-04-15 8:30 ` [PATCH for-5.1 3/5] qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead Markus Armbruster
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2020-04-15 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qobject/qjson.c | 40 ++++++++++++++++------------------------
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/qobject/qjson.c b/qobject/qjson.c
index db36101f3b..f3c62711b9 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -159,21 +159,28 @@ typedef struct ToJsonIterState
static void to_json(const QObject *obj, QString *str, int pretty, int indent);
+static void json_pretty_newline(QString *str, bool pretty, int indent)
+{
+ int i;
+
+ if (pretty) {
+ qstring_append(str, "\n");
+ for (i = 0 ; i < indent ; i++) {
+ qstring_append(str, " ");
+ }
+ }
+}
+
static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
{
ToJsonIterState *s = opaque;
QString *qkey;
- int j;
if (s->count) {
qstring_append(s->str, s->pretty ? "," : ", ");
}
- if (s->pretty) {
- qstring_append(s->str, "\n");
- for (j = 0 ; j < s->indent ; j++)
- qstring_append(s->str, " ");
- }
+ json_pretty_newline(s->str, s->pretty, s->indent);
qkey = qstring_from_str(key);
to_json(QOBJECT(qkey), s->str, s->pretty, s->indent);
@@ -187,17 +194,12 @@ static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
static void to_json_list_iter(QObject *obj, void *opaque)
{
ToJsonIterState *s = opaque;
- int j;
if (s->count) {
qstring_append(s->str, s->pretty ? "," : ", ");
}
- if (s->pretty) {
- qstring_append(s->str, "\n");
- for (j = 0 ; j < s->indent ; j++)
- qstring_append(s->str, " ");
- }
+ json_pretty_newline(s->str, s->pretty, s->indent);
to_json(obj, s->str, s->pretty, s->indent);
s->count++;
@@ -282,12 +284,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
s.pretty = pretty;
qstring_append(str, "{");
qdict_iter(val, to_json_dict_iter, &s);
- if (pretty) {
- int j;
- qstring_append(str, "\n");
- for (j = 0 ; j < indent ; j++)
- qstring_append(str, " ");
- }
+ json_pretty_newline(str, pretty, indent);
qstring_append(str, "}");
break;
}
@@ -301,12 +298,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
s.pretty = pretty;
qstring_append(str, "[");
qlist_iter(val, (void *)to_json_list_iter, &s);
- if (pretty) {
- int j;
- qstring_append(str, "\n");
- for (j = 0 ; j < indent ; j++)
- qstring_append(str, " ");
- }
+ json_pretty_newline(str, pretty, indent);
qstring_append(str, "]");
break;
}
--
2.21.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH for-5.1 2/5] qobject: Factor out helper json_pretty_newline()
2020-04-15 8:30 ` [PATCH for-5.1 2/5] qobject: Factor out helper json_pretty_newline() Markus Armbruster
@ 2020-04-15 12:28 ` Eric Blake
2020-04-15 14:46 ` Markus Armbruster
0 siblings, 1 reply; 14+ messages in thread
From: Eric Blake @ 2020-04-15 12:28 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mdroth
On 4/15/20 3:30 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> qobject/qjson.c | 40 ++++++++++++++++------------------------
> 1 file changed, 16 insertions(+), 24 deletions(-)
>
> diff --git a/qobject/qjson.c b/qobject/qjson.c
> index db36101f3b..f3c62711b9 100644
> --- a/qobject/qjson.c
> +++ b/qobject/qjson.c
> @@ -159,21 +159,28 @@ typedef struct ToJsonIterState
>
> static void to_json(const QObject *obj, QString *str, int pretty, int indent);
>
> +static void json_pretty_newline(QString *str, bool pretty, int indent)
> +{
> + int i;
> +
> + if (pretty) {
> + qstring_append(str, "\n");
> + for (i = 0 ; i < indent ; i++) {
Why are you keeping the spaces before ; ? Yes, I know they were
copied-and-pasted from the old code, but as long as you are refactoring,
fixing the style is worthwhile.
Otherwise,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH for-5.1 2/5] qobject: Factor out helper json_pretty_newline()
2020-04-15 12:28 ` Eric Blake
@ 2020-04-15 14:46 ` Markus Armbruster
0 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-04-15 14:46 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, mdroth
Eric Blake <eblake@redhat.com> writes:
> On 4/15/20 3:30 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> qobject/qjson.c | 40 ++++++++++++++++------------------------
>> 1 file changed, 16 insertions(+), 24 deletions(-)
>>
>> diff --git a/qobject/qjson.c b/qobject/qjson.c
>> index db36101f3b..f3c62711b9 100644
>> --- a/qobject/qjson.c
>> +++ b/qobject/qjson.c
>> @@ -159,21 +159,28 @@ typedef struct ToJsonIterState
>> static void to_json(const QObject *obj, QString *str, int
>> pretty, int indent);
>> +static void json_pretty_newline(QString *str, bool pretty, int
>> indent)
>> +{
>> + int i;
>> +
>> + if (pretty) {
>> + qstring_append(str, "\n");
>> + for (i = 0 ; i < indent ; i++) {
>
> Why are you keeping the spaces before ; ? Yes, I know they were
> copied-and-pasted from the old code, but as long as you are
> refactoring, fixing the style is worthwhile.
Makes sense.
> Otherwise,
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH for-5.1 3/5] qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead
2020-04-15 8:30 [PATCH for-5.1 0/5] qobject: Minor spring cleaning Markus Armbruster
2020-04-15 8:30 ` [PATCH for-5.1 1/5] qobject: Clean up QLIST_FOREACH_ENTRY() Markus Armbruster
2020-04-15 8:30 ` [PATCH for-5.1 2/5] qobject: Factor out helper json_pretty_newline() Markus Armbruster
@ 2020-04-15 8:30 ` Markus Armbruster
2020-04-15 12:31 ` Eric Blake
2020-04-15 8:30 ` [PATCH for-5.1 4/5] qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next() Markus Armbruster
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2020-04-15 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth
qlist_iter() has just three uses outside tests/. Replace by
QLIST_FOREACH_ENTRY() for more concise code and less type punning.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/qapi/qmp/qlist.h | 2 --
qobject/qjson.c | 31 ++++++++++------------------
qobject/qlist.c | 44 +++++++++++-----------------------------
tests/check-qlist.c | 37 +++++++++++++--------------------
4 files changed, 37 insertions(+), 77 deletions(-)
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 07ecae81e4..595b7943e1 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -47,8 +47,6 @@ static inline QObject *qlist_entry_obj(const QListEntry *entry)
QList *qlist_new(void);
QList *qlist_copy(QList *src);
void qlist_append_obj(QList *qlist, QObject *obj);
-void qlist_iter(const QList *qlist,
- void (*iter)(QObject *obj, void *opaque), void *opaque);
QObject *qlist_pop(QList *qlist);
QObject *qlist_peek(QList *qlist);
int qlist_empty(const QList *qlist);
diff --git a/qobject/qjson.c b/qobject/qjson.c
index f3c62711b9..10050354a0 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -191,20 +191,6 @@ static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
s->count++;
}
-static void to_json_list_iter(QObject *obj, void *opaque)
-{
- ToJsonIterState *s = opaque;
-
- if (s->count) {
- qstring_append(s->str, s->pretty ? "," : ", ");
- }
-
- json_pretty_newline(s->str, s->pretty, s->indent);
-
- to_json(obj, s->str, s->pretty, s->indent);
- s->count++;
-}
-
static void to_json(const QObject *obj, QString *str, int pretty, int indent)
{
switch (qobject_type(obj)) {
@@ -289,15 +275,20 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
break;
}
case QTYPE_QLIST: {
- ToJsonIterState s;
QList *val = qobject_to(QList, obj);
+ const char *comma = pretty ? "," : ", ";
+ const char *sep = "";
+ QListEntry *entry;
- s.count = 0;
- s.str = str;
- s.indent = indent + 1;
- s.pretty = pretty;
qstring_append(str, "[");
- qlist_iter(val, (void *)to_json_list_iter, &s);
+
+ QLIST_FOREACH_ENTRY(val, entry) {
+ qstring_append(str, sep);
+ json_pretty_newline(str, pretty, indent + 1);
+ to_json(qlist_entry_obj(entry), str, pretty, indent + 1);
+ sep = comma;
+ }
+
json_pretty_newline(str, pretty, indent);
qstring_append(str, "]");
break;
diff --git a/qobject/qlist.c b/qobject/qlist.c
index b3274af88b..1be95367d1 100644
--- a/qobject/qlist.c
+++ b/qobject/qlist.c
@@ -34,20 +34,17 @@ QList *qlist_new(void)
return qlist;
}
-static void qlist_copy_elem(QObject *obj, void *opaque)
-{
- QList *dst = opaque;
-
- qobject_ref(obj);
- qlist_append_obj(dst, obj);
-}
-
QList *qlist_copy(QList *src)
{
QList *dst = qlist_new();
+ QListEntry *entry;
+ QObject *elt;
- qlist_iter(src, qlist_copy_elem, dst);
-
+ QLIST_FOREACH_ENTRY(src, entry) {
+ elt = qlist_entry_obj(entry);
+ qobject_ref(elt);
+ qlist_append_obj(dst, elt);
+ }
return dst;
}
@@ -86,21 +83,6 @@ void qlist_append_null(QList *qlist)
qlist_append(qlist, qnull());
}
-/**
- * qlist_iter(): Iterate over all the list's stored values.
- *
- * This function allows the user to provide an iterator, which will be
- * called for each stored value in the list.
- */
-void qlist_iter(const QList *qlist,
- void (*iter)(QObject *obj, void *opaque), void *opaque)
-{
- QListEntry *entry;
-
- QTAILQ_FOREACH(entry, &qlist->head, next)
- iter(entry->value, opaque);
-}
-
QObject *qlist_pop(QList *qlist)
{
QListEntry *entry;
@@ -137,16 +119,14 @@ int qlist_empty(const QList *qlist)
return QTAILQ_EMPTY(&qlist->head);
}
-static void qlist_size_iter(QObject *obj, void *opaque)
-{
- size_t *count = opaque;
- (*count)++;
-}
-
size_t qlist_size(const QList *qlist)
{
size_t count = 0;
- qlist_iter(qlist, qlist_size_iter, &count);
+ QListEntry *entry;
+
+ QLIST_FOREACH_ENTRY(qlist, entry) {
+ count++;
+ }
return count;
}
diff --git a/tests/check-qlist.c b/tests/check-qlist.c
index ece83e293d..3cd0ccbf19 100644
--- a/tests/check-qlist.c
+++ b/tests/check-qlist.c
@@ -61,40 +61,31 @@ static void qobject_to_qlist_test(void)
qobject_unref(qlist);
}
-static int iter_called;
-static const int iter_max = 42;
-
-static void iter_func(QObject *obj, void *opaque)
-{
- QNum *qi;
- int64_t val;
-
- g_assert(opaque == NULL);
-
- qi = qobject_to(QNum, obj);
- g_assert(qi != NULL);
-
- g_assert(qnum_get_try_int(qi, &val));
- g_assert_cmpint(val, >=, 0);
- g_assert_cmpint(val, <=, iter_max);
-
- iter_called++;
-}
-
static void qlist_iter_test(void)
{
+ const int iter_max = 42;
int i;
QList *qlist;
+ QListEntry *entry;
+ QNum *qi;
+ int64_t val;
qlist = qlist_new();
for (i = 0; i < iter_max; i++)
qlist_append_int(qlist, i);
- iter_called = 0;
- qlist_iter(qlist, iter_func, NULL);
+ i = 0;
+ QLIST_FOREACH_ENTRY(qlist, entry) {
+ qi = qobject_to(QNum, qlist_entry_obj(entry));
+ g_assert(qi != NULL);
- g_assert(iter_called == iter_max);
+ g_assert(qnum_get_try_int(qi, &val));
+ g_assert_cmpint(val, ==, i);
+ i++;
+ }
+
+ g_assert(i == iter_max);
qobject_unref(qlist);
}
--
2.21.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH for-5.1 3/5] qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead
2020-04-15 8:30 ` [PATCH for-5.1 3/5] qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead Markus Armbruster
@ 2020-04-15 12:31 ` Eric Blake
0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2020-04-15 12:31 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mdroth
On 4/15/20 3:30 AM, Markus Armbruster wrote:
> qlist_iter() has just three uses outside tests/. Replace by
> QLIST_FOREACH_ENTRY() for more concise code and less type punning.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> include/qapi/qmp/qlist.h | 2 --
> qobject/qjson.c | 31 ++++++++++------------------
> qobject/qlist.c | 44 +++++++++++-----------------------------
> tests/check-qlist.c | 37 +++++++++++++--------------------
> 4 files changed, 37 insertions(+), 77 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH for-5.1 4/5] qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next()
2020-04-15 8:30 [PATCH for-5.1 0/5] qobject: Minor spring cleaning Markus Armbruster
` (2 preceding siblings ...)
2020-04-15 8:30 ` [PATCH for-5.1 3/5] qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead Markus Armbruster
@ 2020-04-15 8:30 ` Markus Armbruster
2020-04-15 12:34 ` Eric Blake
2020-04-15 8:30 ` [PATCH for-5.1 5/5] qemu-option: Clean up after the previous commit Markus Armbruster
2020-04-29 7:46 ` [PATCH for-5.1 0/5] qobject: Minor spring cleaning Markus Armbruster
5 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2020-04-15 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth
qdict_iter() has just three uses and no test coverage. Replace by
qdict_first(), qdict_next() for more concise code and less type
punning.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/qapi/qmp/qdict.h | 3 --
qapi/qobject-input-visitor.c | 21 +++++++-------
qobject/qdict.c | 19 -------------
qobject/qjson.c | 54 +++++++++++++-----------------------
util/qemu-option.c | 10 ++++++-
5 files changed, 40 insertions(+), 67 deletions(-)
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 7f3ec10a10..da942347a7 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -40,9 +40,6 @@ void qdict_del(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);
-void qdict_iter(const QDict *qdict,
- void (*iter)(const char *key, QObject *obj, void *opaque),
- void *opaque);
const QDictEntry *qdict_first(const QDict *qdict);
const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry);
void qdict_destroy_obj(QObject *obj);
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 32236cbcb1..5ce3ec2e5f 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -203,31 +203,32 @@ static const char *qobject_input_get_keyval(QObjectInputVisitor *qiv,
return qstring_get_str(qstr);
}
-static void qdict_add_key(const char *key, QObject *obj, void *opaque)
-{
- GHashTable *h = opaque;
- g_hash_table_insert(h, (gpointer) key, NULL);
-}
-
static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
const char *name,
QObject *obj, void *qapi)
{
GHashTable *h;
StackObject *tos = g_new0(StackObject, 1);
+ QDict *qdict = qobject_to(QDict, obj);
+ QList *qlist = qobject_to(QList, obj);
+ const QDictEntry *entry;
assert(obj);
tos->name = name;
tos->obj = obj;
tos->qapi = qapi;
- if (qobject_type(obj) == QTYPE_QDICT) {
+ if (qdict) {
h = g_hash_table_new(g_str_hash, g_str_equal);
- qdict_iter(qobject_to(QDict, obj), qdict_add_key, h);
+ for (entry = qdict_first(qdict);
+ entry;
+ entry = qdict_next(qdict, entry)) {
+ g_hash_table_insert(h, (void *)qdict_entry_key(entry), NULL);
+ }
tos->h = h;
} else {
- assert(qobject_type(obj) == QTYPE_QLIST);
- tos->entry = qlist_first(qobject_to(QList, obj));
+ assert(qlist);
+ tos->entry = qlist_first(qlist);
tos->index = -1;
}
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 3d8c2f7bbc..526de54ceb 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -298,25 +298,6 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key)
return qstr ? qstring_get_str(qstr) : NULL;
}
-/**
- * qdict_iter(): Iterate over all the dictionary's stored values.
- *
- * This function allows the user to provide an iterator, which will be
- * called for each stored value in the dictionary.
- */
-void qdict_iter(const QDict *qdict,
- void (*iter)(const char *key, QObject *obj, void *opaque),
- void *opaque)
-{
- int i;
- QDictEntry *entry;
-
- for (i = 0; i < QDICT_BUCKET_MAX; i++) {
- QLIST_FOREACH(entry, &qdict->table[i], next)
- iter(entry->key, entry->value, opaque);
- }
-}
-
static QDictEntry *qdict_next_entry(const QDict *qdict, int first_bucket)
{
int i;
diff --git a/qobject/qjson.c b/qobject/qjson.c
index 10050354a0..2a3336e459 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -149,14 +149,6 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...)
return qdict;
}
-typedef struct ToJsonIterState
-{
- int indent;
- int pretty;
- int count;
- QString *str;
-} ToJsonIterState;
-
static void to_json(const QObject *obj, QString *str, int pretty, int indent);
static void json_pretty_newline(QString *str, bool pretty, int indent)
@@ -171,26 +163,6 @@ static void json_pretty_newline(QString *str, bool pretty, int indent)
}
}
-static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
-{
- ToJsonIterState *s = opaque;
- QString *qkey;
-
- if (s->count) {
- qstring_append(s->str, s->pretty ? "," : ", ");
- }
-
- json_pretty_newline(s->str, s->pretty, s->indent);
-
- qkey = qstring_from_str(key);
- to_json(QOBJECT(qkey), s->str, s->pretty, s->indent);
- qobject_unref(qkey);
-
- qstring_append(s->str, ": ");
- to_json(obj, s->str, s->pretty, s->indent);
- s->count++;
-}
-
static void to_json(const QObject *obj, QString *str, int pretty, int indent)
{
switch (qobject_type(obj)) {
@@ -261,15 +233,29 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
break;
}
case QTYPE_QDICT: {
- ToJsonIterState s;
QDict *val = qobject_to(QDict, obj);
+ const char *comma = pretty ? "," : ", ";
+ const char *sep = "";
+ const QDictEntry *entry;
+ QString *qkey;
- s.count = 0;
- s.str = str;
- s.indent = indent + 1;
- s.pretty = pretty;
qstring_append(str, "{");
- qdict_iter(val, to_json_dict_iter, &s);
+
+ for (entry = qdict_first(val);
+ entry;
+ entry = qdict_next(val, entry)) {
+ qstring_append(str, sep);
+ json_pretty_newline(str, pretty, indent + 1);
+
+ qkey = qstring_from_str(qdict_entry_key(entry));
+ to_json(QOBJECT(qkey), str, pretty, indent + 1);
+ qobject_unref(qkey);
+
+ qstring_append(str, ": ");
+ to_json(qdict_entry_value(entry), str, pretty, indent + 1);
+ sep = comma;
+ }
+
json_pretty_newline(str, pretty, indent);
qstring_append(str, "}");
break;
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 97172b5eaa..3fabfd157d 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1015,6 +1015,7 @@ QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
OptsFromQDictState state;
Error *local_err = NULL;
QemuOpts *opts;
+ const QDictEntry *entry;
opts = qemu_opts_create(list, qdict_get_try_str(qdict, "id"), 1,
&local_err);
@@ -1027,7 +1028,14 @@ QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
state.errp = &local_err;
state.opts = opts;
- qdict_iter(qdict, qemu_opts_from_qdict_1, &state);
+
+ for (entry = qdict_first(qdict);
+ entry;
+ entry = qdict_next(qdict, entry)) {
+ qemu_opts_from_qdict_1(qdict_entry_key(entry),
+ qdict_entry_value(entry),
+ &state);
+ }
if (local_err) {
error_propagate(errp, local_err);
qemu_opts_del(opts);
--
2.21.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH for-5.1 4/5] qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next()
2020-04-15 8:30 ` [PATCH for-5.1 4/5] qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next() Markus Armbruster
@ 2020-04-15 12:34 ` Eric Blake
2020-04-15 14:48 ` Markus Armbruster
0 siblings, 1 reply; 14+ messages in thread
From: Eric Blake @ 2020-04-15 12:34 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mdroth
On 4/15/20 3:30 AM, Markus Armbruster wrote:
> qdict_iter() has just three uses and no test coverage. Replace by
> qdict_first(), qdict_next() for more concise code and less type
> punning.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> include/qapi/qmp/qdict.h | 3 --
> qapi/qobject-input-visitor.c | 21 +++++++-------
> qobject/qdict.c | 19 -------------
> qobject/qjson.c | 54 +++++++++++++-----------------------
> util/qemu-option.c | 10 ++++++-
> 5 files changed, 40 insertions(+), 67 deletions(-)
>
> static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
> const char *name,
> QObject *obj, void *qapi)
> {
> GHashTable *h;
> StackObject *tos = g_new0(StackObject, 1);
> + QDict *qdict = qobject_to(QDict, obj);
> + QList *qlist = qobject_to(QList, obj);
> + const QDictEntry *entry;
>
> assert(obj);
> tos->name = name;
> tos->obj = obj;
> tos->qapi = qapi;
>
> - if (qobject_type(obj) == QTYPE_QDICT) {
> + if (qdict) {
> h = g_hash_table_new(g_str_hash, g_str_equal);
> - qdict_iter(qobject_to(QDict, obj), qdict_add_key, h);
> + for (entry = qdict_first(qdict);
> + entry;
> + entry = qdict_next(qdict, entry)) {
> + g_hash_table_insert(h, (void *)qdict_entry_key(entry), NULL);
Is the cast to void* necessary?
Otherwise,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH for-5.1 4/5] qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next()
2020-04-15 12:34 ` Eric Blake
@ 2020-04-15 14:48 ` Markus Armbruster
0 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-04-15 14:48 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, mdroth
Eric Blake <eblake@redhat.com> writes:
> On 4/15/20 3:30 AM, Markus Armbruster wrote:
>> qdict_iter() has just three uses and no test coverage. Replace by
>> qdict_first(), qdict_next() for more concise code and less type
>> punning.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> include/qapi/qmp/qdict.h | 3 --
>> qapi/qobject-input-visitor.c | 21 +++++++-------
>> qobject/qdict.c | 19 -------------
>> qobject/qjson.c | 54 +++++++++++++-----------------------
>> util/qemu-option.c | 10 ++++++-
>> 5 files changed, 40 insertions(+), 67 deletions(-)
>>
>
>> static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
>> const char *name,
>> QObject *obj, void *qapi)
>> {
>> GHashTable *h;
>> StackObject *tos = g_new0(StackObject, 1);
>> + QDict *qdict = qobject_to(QDict, obj);
>> + QList *qlist = qobject_to(QList, obj);
>> + const QDictEntry *entry;
>> assert(obj);
>> tos->name = name;
>> tos->obj = obj;
>> tos->qapi = qapi;
>> - if (qobject_type(obj) == QTYPE_QDICT) {
>> + if (qdict) {
>> h = g_hash_table_new(g_str_hash, g_str_equal);
>> - qdict_iter(qobject_to(QDict, obj), qdict_add_key, h);
>> + for (entry = qdict_first(qdict);
>> + entry;
>> + entry = qdict_next(qdict, entry)) {
>> + g_hash_table_insert(h, (void *)qdict_entry_key(entry), NULL);
>
> Is the cast to void* necessary?
It casts away const.
> Otherwise,
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH for-5.1 5/5] qemu-option: Clean up after the previous commit
2020-04-15 8:30 [PATCH for-5.1 0/5] qobject: Minor spring cleaning Markus Armbruster
` (3 preceding siblings ...)
2020-04-15 8:30 ` [PATCH for-5.1 4/5] qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next() Markus Armbruster
@ 2020-04-15 8:30 ` Markus Armbruster
2020-04-15 12:35 ` Eric Blake
2020-04-29 7:46 ` [PATCH for-5.1 0/5] qobject: Minor spring cleaning Markus Armbruster
5 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2020-04-15 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
util/qemu-option.c | 43 +++++++++++++++----------------------------
1 file changed, 15 insertions(+), 28 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 3fabfd157d..123c52bf07 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -967,18 +967,16 @@ void qemu_opts_set_defaults(QemuOptsList *list, const char *params,
assert(opts);
}
-typedef struct OptsFromQDictState {
- QemuOpts *opts;
- Error **errp;
-} OptsFromQDictState;
-
-static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
+static void qemu_opts_from_qdict_entry(QemuOpts *opts,
+ const QDictEntry *entry,
+ Error **errp)
{
- OptsFromQDictState *state = opaque;
+ const char *key = qdict_entry_key(entry);
+ QObject *obj = qdict_entry_value(entry);
char buf[32], *tmp = NULL;
const char *value;
- if (!strcmp(key, "id") || *state->errp) {
+ if (!strcmp(key, "id")) {
return;
}
@@ -999,7 +997,7 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
return;
}
- qemu_opt_set(state->opts, key, value, state->errp);
+ qemu_opt_set(opts, key, value, errp);
g_free(tmp);
}
@@ -1012,7 +1010,6 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
Error **errp)
{
- OptsFromQDictState state;
Error *local_err = NULL;
QemuOpts *opts;
const QDictEntry *entry;
@@ -1026,20 +1023,15 @@ QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
assert(opts != NULL);
- state.errp = &local_err;
- state.opts = opts;
-
for (entry = qdict_first(qdict);
entry;
entry = qdict_next(qdict, entry)) {
- qemu_opts_from_qdict_1(qdict_entry_key(entry),
- qdict_entry_value(entry),
- &state);
- }
- if (local_err) {
- error_propagate(errp, local_err);
- qemu_opts_del(opts);
- return NULL;
+ qemu_opts_from_qdict_entry(opts, entry, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ qemu_opts_del(opts);
+ return NULL;
+ }
}
return opts;
@@ -1058,21 +1050,16 @@ void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp)
while (entry != NULL) {
Error *local_err = NULL;
- OptsFromQDictState state = {
- .errp = &local_err,
- .opts = opts,
- };
next = qdict_next(qdict, entry);
if (find_desc_by_name(opts->list->desc, entry->key)) {
- qemu_opts_from_qdict_1(entry->key, entry->value, &state);
+ qemu_opts_from_qdict_entry(opts, entry, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
- } else {
- qdict_del(qdict, entry->key);
}
+ qdict_del(qdict, entry->key);
}
entry = next;
--
2.21.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH for-5.1 0/5] qobject: Minor spring cleaning
2020-04-15 8:30 [PATCH for-5.1 0/5] qobject: Minor spring cleaning Markus Armbruster
` (4 preceding siblings ...)
2020-04-15 8:30 ` [PATCH for-5.1 5/5] qemu-option: Clean up after the previous commit Markus Armbruster
@ 2020-04-29 7:46 ` Markus Armbruster
5 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-04-29 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth
Queued.
^ permalink raw reply [flat|nested] 14+ messages in thread