On 2018-02-24 22:04, Eric Blake wrote: > On 02/24/2018 09:40 AM, Max Reitz wrote: >> This patch was generated using the following Coccinelle script: >> > >> and a bit of manual fix-up for overly long lines and three places in >> tests/check-qjson.c that Coccinelle did not find. >> >> Signed-off-by: Max Reitz <mre...@redhat.com> >> Reviewed-by: Alberto Garcia <be...@igalia.com> >> --- > >> diff --git a/block.c b/block.c >> index 814e5a02da..cb69fd7ae4 100644 >> --- a/block.c >> +++ b/block.c >> @@ -1457,7 +1457,7 @@ static QDict *parse_json_filename(const char >> *filename, Error **errp) >> return NULL; >> } >> - options = qobject_to_qdict(options_obj); >> + options = qobject_to(options_obj, QDict); > > Bikeshedding - would it read any easier as: > > options = qobject_to(QDict, options_obj); > > ? If so, your Coccinelle script can be touched up, and patch 2/7 swaps > argument order around, so it would be tolerable but still slightly > busywork to regenerate the series. But I'm not strongly attached to > either order, and so I'm also willing to take this as-is (especially > since that's less work), if no one else has a strong opinion that > swapping order would aid legibility.
Well, same for me. :-)
In a template/generic language, we'd write the type first (e.g.
qobject_cast<QDict>(options_obj)). But maybe we'd write the object
first, too (e.g. options_obj.cast<QDict>()). And the current order of
the arguments follows the order in the name ("qobject" options_obj "to"
QDict). But maybe it's more natural to read it as "qobject to" QDict
"applied to" options_obj.
I don't know either.
Max
> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
>
>> +++ b/block/rbd.c
>> @@ -256,14 +256,14 @@ static int qemu_rbd_set_keypairs(rados_t
>> cluster, const char *keypairs_json,
>> if (!keypairs_json) {
>> return ret;
>> }
>> - keypairs = qobject_to_qlist(qobject_from_json(keypairs_json,
>> - &error_abort));
>> + keypairs = qobject_to(qobject_from_json(keypairs_json,
>> &error_abort),
>> + QList);
>
> The question about legibility gets a bit more obvious when you span lines.
>
>> @@ -893,8 +893,9 @@ static void simple_number(void)
>> QNum *qnum;
>> int64_t val;
>> - qnum =
>> qobject_to_qnum(qobject_from_json(test_cases[i].encoded,
>> - &error_abort));
>> + qnum = qobject_to(qobject_from_json(test_cases[i].encoded,
>> + &error_abort),
>> + QNum);
>
signature.asc
Description: OpenPGP digital signature
