Re: [Qemu-block] [PATCH v3 3/7] qapi: Replace qobject_to_X(o) by qobject_to(o, X)

[Max Reitz]

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 <address@hidden>
>> Reviewed-by: Alberto Garcia <address@hidden>
>> ---
> 
>> 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 <address@hidden>
> 
> 
>> +++ 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