Markus Armbruster <arm...@redhat.com> writes:

> Markus Armbruster <arm...@redhat.com> writes:
>
>> Markus Armbruster <arm...@redhat.com> writes:
>>
>>> "Daniel P. Berrange" <berra...@redhat.com> 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 <kw...@redhat.com>
>>>> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>>>> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
>> [...]
>>>> 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.

[...]

Reply via email to