From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47382) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ai95Q-0006iw-SM for qemu-devel@nongnu.org; Mon, 21 Mar 2016 19:18:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ai95P-0005dT-DY for qemu-devel@nongnu.org; Mon, 21 Mar 2016 19:18:12 -0400 References: <1457635927-23045-1-git-send-email-berrange@redhat.com> <1457636396-24983-1-git-send-email-berrange@redhat.com> <1457636396-24983-2-git-send-email-berrange@redhat.com> From: Eric Blake Message-ID: <56F08129.60807@redhat.com> Date: Mon, 21 Mar 2016 17:18:01 -0600 MIME-Version: 1.0 In-Reply-To: <1457636396-24983-2-git-send-email-berrange@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="jId6XbJJRdFcqDQqgrAXG2s4uQAqsNNUp" Subject: Re: [Qemu-devel] [PATCH v3 02/10] qapi: allow QmpInputVisitor to auto-cast types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Paolo Bonzini , qemu-block@nongnu.org, Markus Armbruster , =?UTF-8?Q?Andreas_F=c3=a4rber?= , Max Reitz This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --jId6XbJJRdFcqDQqgrAXG2s4uQAqsNNUp Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 03/10/2016 11:59 AM, Daniel P. Berrange wrote: > Currently the QmpInputVisitor assumes that all scalar > values are directly represented as their final types. > ie it assumes an 'int' is using QInt, and a 'bool' is > using QBool. >=20 > This extends it so that QString is optionally permitted > for any of the non-string scalar types. This behaviour > is turned on by requesting the 'autocast' flag in the > constructor. >=20 > This makes it possible to use QmpInputVisitor with a > QDict produced from QemuOpts, where everything is in > string format. >=20 > Signed-off-by: Daniel P. Berrange > --- > include/qapi/qmp-input-visitor.h | 3 + > qapi/qmp-input-visitor.c | 96 +++++++++++++++++++++++++++----= - > tests/test-qmp-input-visitor.c | 115 +++++++++++++++++++++++++++++++= +++++++- > 3 files changed, 196 insertions(+), 18 deletions(-) >=20 > diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-= visitor.h > index 3ed499c..c25cb7c 100644 > --- a/include/qapi/qmp-input-visitor.h > +++ b/include/qapi/qmp-input-visitor.h > @@ -21,6 +21,9 @@ typedef struct QmpInputVisitor QmpInputVisitor; > =20 > QmpInputVisitor *qmp_input_visitor_new(QObject *obj); > QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj); > +QmpInputVisitor *qmp_input_visitor_new_full(QObject *obj, > + bool strict, > + bool autocast); We have so few uses of qmp_input_visitor_new* that it might be worth just having a single prototype, and maybe using an 'int flags' instead of a string of bool. But not a show-stopper for this patch (rather, an idea for a future patch). > - *obj =3D qint_get_int(qint); > + qstr =3D qobject_to_qstring(qobj); > + if (qstr && qstr->string && qiv->autocast) { > + errno =3D 0; Dead setting of errno, since... > + if (qemu_strtoll(qstr->string, NULL, 10, obj) =3D=3D 0) { qemu_strtoll() handles it on your behalf, and you aren't using error_setg_errno(). > @@ -233,30 +245,61 @@ static void qmp_input_type_uint64(Visitor *v, con= st char *name, uint64_t *obj, > { > /* FIXME: qobject_to_qint mishandles values over INT64_MAX */ > QmpInputVisitor *qiv =3D to_qiv(v); > - QInt *qint =3D qobject_to_qint(qmp_input_get_object(qiv, name, tru= e)); > + QObject *qobj =3D qmp_input_get_object(qiv, name, true); > + QInt *qint; > + QString *qstr; > =20 > - if (!qint) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "n= ull", > - "integer"); > + qint =3D qobject_to_qint(qobj); > + if (qint) { > + *obj =3D qint_get_int(qint); > return; > } > =20 > - *obj =3D qint_get_int(qint); > + qstr =3D qobject_to_qstring(qobj); > + if (qstr && qstr->string && qiv->autocast) { > + errno =3D 0; > + if (qemu_strtoull(qstr->string, NULL, 10, obj) =3D=3D 0) { And again. Hmm. Do we need to worry about partial asymmetry? That is, qint_get_int() returns a signed number, but qemu_strtoull() parses unsigned; if the original conversion from JSON to qint uses a different parser, then we could have funny results where we get different results for things like: "key1":9223372036854775807, "key2":"9223372036854775807", even though the same string of digits is being parsed, based on whether the different parsers handle numbers larger than INT64_MAX differently. [Ultimately, I'd like QInt to be enhanced to track whether the input was signed or unsigned, and automatically make the output match the input when converting back to string; that is, track 65 bits of information instead of 64; but that's no sooner than 2.7 material] > static void qmp_input_type_bool(Visitor *v, const char *name, bool *ob= j, > Error **errp) > { > QmpInputVisitor *qiv =3D to_qiv(v); > - QBool *qbool =3D qobject_to_qbool(qmp_input_get_object(qiv, name, = true)); > + QObject *qobj =3D qmp_input_get_object(qiv, name, true); > + QBool *qbool; > + QString *qstr; > =20 > - if (!qbool) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "n= ull", > - "boolean"); > + qbool =3D qobject_to_qbool(qobj); > + if (qbool) { > + *obj =3D qbool_get_bool(qbool); > return; > } > =20 > - *obj =3D qbool_get_bool(qbool); > + > + qstr =3D qobject_to_qstring(qobj); > + if (qstr && qstr->string && qiv->autocast) { > + if (!strcasecmp(qstr->string, "on") || > + !strcasecmp(qstr->string, "yes") || > + !strcasecmp(qstr->string, "true")) { > + *obj =3D true; > + return; > + } Do we also want to allow "0"/"1" for true/false? Overall, I'm a big fan of this patch. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --jId6XbJJRdFcqDQqgrAXG2s4uQAqsNNUp Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJW8IEpAAoJEKeha0olJ0NqDMMH/jSqCRLmypcotawN7Lreuy4G wmqnB9TC1ZNnJs/5HpEZ8eiXEbUm+IiF1XgIGIzkrgV78RPH2rmTaLcDJbXnLkhW 5KrMOYl7lLwDvqE+stFxwAKSE3LM1JlRADGK/uHSMBxmPRwEhIwdLH6T34Vt/5I7 VaZCe9RFLBeHjv4YL33DMTk5Z7Q/xcfOkaQGp4H61UEwtZLSAF++lZScjpSW173G eqA64ocXjSWU97miQ3yazG09hTIwHZE56BH1JiYRGn0jx82bMD5PdiPo0gTFjTK1 zE1WMpYyQJdqTfMbnF/GgnX/DVgu/L7ohc94r8JS6frCwOqpMsIIFQ+NLWDp8Q0= =95jS -----END PGP SIGNATURE----- --jId6XbJJRdFcqDQqgrAXG2s4uQAqsNNUp--