[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v14 07/21] qapi: permit scalar type
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v14 07/21] qapi: permit scalar type conversions in QObjectInputVisitor |
Date: |
Wed, 12 Oct 2016 17:26:13 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Markus Armbruster <address@hidden> writes:
> Markus Armbruster <address@hidden> writes:
>
>> Markus Armbruster <address@hidden> writes:
>>
>>> "Daniel P. Berrange" <address@hidden> writes:
>>>
>>>> Currently the QObjectInputVisitor assumes that all scalar
>>>> values are directly represented as the final types declared
>>>> by the thing being visited. ie it assumes an 'int' is using
>>>
>>> i.e.
>>>
>>>> QInt, and a 'bool' is using QBool, etc. This is good when
>>>> QObjectInputVisitor is fed a QObject that came from a JSON
>>>> document on the QMP monitor, as it will strictly validate
>>>> correctness.
>>>>
>>>> To allow QObjectInputVisitor to be reused for visiting
>>>> a QObject originating from QemuOpts, an alternative mode
>>>> is needed where all the scalars types are represented as
>>>> QString and converted on the fly to the final desired
>>>> type.
>>>>
>>>> Reviewed-by: Kevin Wolf <address@hidden>
>>>> Reviewed-by: Marc-André Lureau <address@hidden>
>>>> Signed-off-by: Daniel P. Berrange <address@hidden>
>> [...]
>>>> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
>>>> index 5ff3db3..cf41df6 100644
>>>> --- a/qapi/qobject-input-visitor.c
>>>> +++ b/qapi/qobject-input-visitor.c
>>>> @@ -20,6 +20,7 @@
>>>> #include "qemu-common.h"
>>>> #include "qapi/qmp/types.h"
>>>> #include "qapi/qmp/qerror.h"
>>>> +#include "qemu/cutils.h"
>>>>
>>>> #define QIV_STACK_SIZE 1024
>>>>
>>>> @@ -263,6 +264,28 @@ static void qobject_input_type_int64(Visitor *v,
>>>> const char *name, int64_t *obj,
>>>> *obj = qint_get_int(qint);
>>>> }
>>>>
>>>> +
>>>> +static void qobject_input_type_int64_autocast(Visitor *v, const char
>>>> *name,
>>>> + int64_t *obj, Error **errp)
>>>> +{
>>>> + QObjectInputVisitor *qiv = to_qiv(v);
>>>> + QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
>>>> + true));
>>>
>>> Needs a rebase for commit 1382d4a. Same elsewhere.
>>>
>>>> + int64_t ret;
>>>> +
>>>> + if (!qstr || !qstr->string) {
>>>
>>> I don't think !qstr->string can happen. Same elsewhere.
>>>
>>>> + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name :
>>>> "null",
>>>> + "string");
>>>> + return;
>>>> + }
>>>
>>> So far, this is basically qobject_input_type_str() less the g_strdup().
>>> Worth factoring out?
>>>
>>> Now we're entering out parsing swamp:
>>>
>>>> +
>>>> + if (qemu_strtoll(qstr->string, NULL, 0, &ret) < 0) {
>>>> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
>>
>> "integer", actually.
>
> Wait! You're using QERR_INVALID_PARAMETER_VALUE here, so it's "an
> integer".
>
> To be pedantically correct, we'd have to complain about the type when
> qemu_strtoll() fails to parse, and about the value when it parses okay,
> but the value is out of range.
Nevermind, I got confused. The type is actually always string here, so
your use of QERR_INVALID_PARAMETER_VALUE is appropriate.
[...]