| Message ID | 20180509165530.29561-8-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/09/2018 11:55 AM, Max Reitz wrote: > The purpose of this function is to prepare a QDict for consumption by > the keyval visitor, which only accepts strings and QNull. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > include/block/qdict.h | 2 ++ > qobject/qdict.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 59 insertions(+) > > +/** > + * Convert all values in a QDict so it can be consumed by the keyval > + * input visitor. QNull values are left as-is, all other values are > + * converted to strings. > + * > + * @qdict must be flattened, i.e. it may not contain any nested QDicts > + * or QLists. Maybe s/flattened/flattened already/ or would it be worth letting this function PERFORM the flattening step automatically? > + */ > +void qdict_stringify_for_keyval(QDict *qdict) > +{ > + const QDictEntry *e; > + > + for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) { > + QString *new_value = NULL; > + > + switch (qobject_type(e->value)) { > + case QTYPE_QNULL: > + /* leave as-is */ > + break; > + > + case QTYPE_QNUM: { > + char *str = qnum_to_string(qobject_to(QNum, e->value)); > + new_value = qstring_from_str(str); > + g_free(str); > + break; > + } > + > + case QTYPE_QSTRING: > + /* leave as-is */ > + break; > + > + case QTYPE_QDICT: > + case QTYPE_QLIST: > + /* @qdict must be flattened */ > + abort(); Matches your documentation about requiring it to be already flattened. > + > + case QTYPE_QBOOL: > + if (qbool_get_bool(qobject_to(QBool, e->value))) { > + new_value = qstring_from_str("on"); > + } else { > + new_value = qstring_from_str("off"); > + } > + break; > + > + case QTYPE_NONE: > + case QTYPE__MAX: > + abort(); > + } > + > + if (new_value) { > + QDictEntry *mut_e = (QDictEntry *)e; A bit of a shame that you have to cast away const. It took me a couple reads to figure out this meant 'mutable_e'. But in the end, the code looks correct. > + qobject_unref(mut_e->value); > + mut_e->value = QOBJECT(new_value); If we're okay requiring the caller to pre-flatten before calling this, instead of offering to do it here, Reviewed-by: Eric Blake <eblake@redhat.com>
On 2018-05-11 20:39, Eric Blake wrote: > On 05/09/2018 11:55 AM, Max Reitz wrote: >> The purpose of this function is to prepare a QDict for consumption by >> the keyval visitor, which only accepts strings and QNull. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> include/block/qdict.h | 2 ++ >> qobject/qdict.c | 57 >> +++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 59 insertions(+) >> > >> +/** >> + * Convert all values in a QDict so it can be consumed by the keyval >> + * input visitor. QNull values are left as-is, all other values are >> + * converted to strings. >> + * >> + * @qdict must be flattened, i.e. it may not contain any nested QDicts >> + * or QLists. > > Maybe s/flattened/flattened already/ > > or would it be worth letting this function PERFORM the flattening step > automatically? Possibly, but I don't think it would be any more efficient, so I'd rather leave it up to the caller to do it explicitly than to do it here and maybe surprise someone. (Indeed I personally prefer to be surprised by an abort() than by some unintended data change.) Max >> + */ >> +void qdict_stringify_for_keyval(QDict *qdict) >> +{ >> + const QDictEntry *e; >> + >> + for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) { >> + QString *new_value = NULL; >> + >> + switch (qobject_type(e->value)) { >> + case QTYPE_QNULL: >> + /* leave as-is */ >> + break; >> + >> + case QTYPE_QNUM: { >> + char *str = qnum_to_string(qobject_to(QNum, e->value)); >> + new_value = qstring_from_str(str); >> + g_free(str); >> + break; >> + } >> + >> + case QTYPE_QSTRING: >> + /* leave as-is */ >> + break; >> + >> + case QTYPE_QDICT: >> + case QTYPE_QLIST: >> + /* @qdict must be flattened */ >> + abort(); > > Matches your documentation about requiring it to be already flattened. > >> + >> + case QTYPE_QBOOL: >> + if (qbool_get_bool(qobject_to(QBool, e->value))) { >> + new_value = qstring_from_str("on"); >> + } else { >> + new_value = qstring_from_str("off"); >> + } >> + break; >> + >> + case QTYPE_NONE: >> + case QTYPE__MAX: >> + abort(); >> + } >> + >> + if (new_value) { >> + QDictEntry *mut_e = (QDictEntry *)e; > > A bit of a shame that you have to cast away const. It took me a couple > reads to figure out this meant 'mutable_e'. But in the end, the code > looks correct. > >> + qobject_unref(mut_e->value); >> + mut_e->value = QOBJECT(new_value); > > If we're okay requiring the caller to pre-flatten before calling this, > instead of offering to do it here, > Reviewed-by: Eric Blake <eblake@redhat.com> >
diff --git a/include/block/qdict.h b/include/block/qdict.h index 825e096a72..05a24677b5 100644 --- a/include/block/qdict.h +++ b/include/block/qdict.h @@ -32,4 +32,6 @@ typedef struct QDictRenames { } QDictRenames; bool qdict_rename_keys(QDict *qdict, const QDictRenames *renames, Error **errp); +void qdict_stringify_for_keyval(QDict *qdict); + #endif diff --git a/qobject/qdict.c b/qobject/qdict.c index 0554c64553..83678c4951 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -1087,3 +1087,60 @@ bool qdict_rename_keys(QDict *qdict, const QDictRenames *renames, Error **errp) } return true; } + +/** + * Convert all values in a QDict so it can be consumed by the keyval + * input visitor. QNull values are left as-is, all other values are + * converted to strings. + * + * @qdict must be flattened, i.e. it may not contain any nested QDicts + * or QLists. + */ +void qdict_stringify_for_keyval(QDict *qdict) +{ + const QDictEntry *e; + + for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) { + QString *new_value = NULL; + + switch (qobject_type(e->value)) { + case QTYPE_QNULL: + /* leave as-is */ + break; + + case QTYPE_QNUM: { + char *str = qnum_to_string(qobject_to(QNum, e->value)); + new_value = qstring_from_str(str); + g_free(str); + break; + } + + case QTYPE_QSTRING: + /* leave as-is */ + break; + + case QTYPE_QDICT: + case QTYPE_QLIST: + /* @qdict must be flattened */ + abort(); + + case QTYPE_QBOOL: + if (qbool_get_bool(qobject_to(QBool, e->value))) { + new_value = qstring_from_str("on"); + } else { + new_value = qstring_from_str("off"); + } + break; + + case QTYPE_NONE: + case QTYPE__MAX: + abort(); + } + + if (new_value) { + QDictEntry *mut_e = (QDictEntry *)e; + qobject_unref(mut_e->value); + mut_e->value = QOBJECT(new_value); + } + } +}
The purpose of this function is to prepare a QDict for consumption by the keyval visitor, which only accepts strings and QNull. Signed-off-by: Max Reitz <mreitz@redhat.com> --- include/block/qdict.h | 2 ++ qobject/qdict.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+)