From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53533) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buLQF-0007Mj-8O for qemu-devel@nongnu.org; Wed, 12 Oct 2016 11:26:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buLQE-0005iA-5F for qemu-devel@nongnu.org; Wed, 12 Oct 2016 11:26:23 -0400 From: Markus Armbruster References: <1475246744-29302-1-git-send-email-berrange@redhat.com> <1475246744-29302-8-git-send-email-berrange@redhat.com> <87d1j6amdm.fsf@dusky.pond.sub.org> <87bmyptybk.fsf@dusky.pond.sub.org> <874m4hsj3y.fsf@dusky.pond.sub.org> Date: Wed, 12 Oct 2016 17:26:13 +0200 In-Reply-To: <874m4hsj3y.fsf@dusky.pond.sub.org> (Markus Armbruster's message of "Wed, 12 Oct 2016 17:05:37 +0200") Message-ID: <87mvi9pp0q.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v14 07/21] qapi: permit scalar type conversions in QObjectInputVisitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Max Reitz , Paolo Bonzini , Andreas =?utf-8?Q?F=C3=A4rber?= Markus Armbruster writes: > Markus Armbruster writes: > >> Markus Armbruster writes: >> >>> "Daniel P. Berrange" 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 >>>> Reviewed-by: Marc-Andr=C3=A9 Lureau >>>> Signed-off-by: Daniel P. Berrange >> [...] >>>> 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" >>>>=20=20 >>>> #define QIV_STACK_SIZE 1024 >>>>=20=20 >>>> @@ -263,6 +264,28 @@ static void qobject_input_type_int64(Visitor *v, = const char *name, int64_t *obj, >>>> *obj =3D qint_get_int(qint); >>>> } >>>>=20=20 >>>> + >>>> +static void qobject_input_type_int64_autocast(Visitor *v, const char = *name, >>>> + int64_t *obj, Error **e= rrp) >>>> +{ >>>> + QObjectInputVisitor *qiv =3D to_qiv(v); >>>> + QString *qstr =3D 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 numbe= r"); >> >> "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. [...]