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 <mre...@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 <ebl...@redhat.com>
>
signature.asc
Description: OpenPGP digital signature
