[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 07/13] qdict: Add qdict_stringify_for_keyval()
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH 07/13] qdict: Add qdict_stringify_for_keyval() |
Date: |
Fri, 11 May 2018 23:42:33 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
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 <address@hidden>
>> ---
>> 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 <address@hidden>
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH 04/13] qapi: Formalize qcow2 encryption probing, (continued)
- [Qemu-block] [PATCH 04/13] qapi: Formalize qcow2 encryption probing, Max Reitz, 2018/05/09
- [Qemu-block] [PATCH 05/13] qapi: Formalize qcow encryption probing, Max Reitz, 2018/05/09
- [Qemu-block] [PATCH 07/13] qdict: Add qdict_stringify_for_keyval(), Max Reitz, 2018/05/09
- [Qemu-block] [PATCH 06/13] block: Add block-specific QDict header, Max Reitz, 2018/05/09
- [Qemu-block] [PATCH 08/13] tests: Add qdict_stringify_for_keyval() test, Max Reitz, 2018/05/09
- [Qemu-block] [PATCH 09/13] qdict: Make qdict_flatten() shallow-clone-friendly, Max Reitz, 2018/05/09
- [Qemu-block] [PATCH 10/13] tests: Add QDict clone-flatten test, Max Reitz, 2018/05/09