From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56344) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZIOsQ-0005ZT-8a for qemu-devel@nongnu.org; Thu, 23 Jul 2015 18:22:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZIOsM-0002E3-Ve for qemu-devel@nongnu.org; Thu, 23 Jul 2015 18:22:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52236) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZIOsM-0002Dv-OD for qemu-devel@nongnu.org; Thu, 23 Jul 2015 18:22:02 -0400 References: <1435782155-31412-1-git-send-email-armbru@redhat.com> <1435782155-31412-42-git-send-email-armbru@redhat.com> From: Eric Blake Message-ID: <55B16903.5080202@redhat.com> Date: Thu, 23 Jul 2015 16:21:55 -0600 MIME-Version: 1.0 In-Reply-To: <1435782155-31412-42-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="TBhlJMkrg4lDCsgOjOaUUaLHclBhp6cHx" Subject: Re: [Qemu-devel] [PATCH RFC v2 41/47] qom: Don't use 'gen': false for qom-get, qom-set, object-add List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: kwolf@redhat.com, berto@igalia.com, mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --TBhlJMkrg4lDCsgOjOaUUaLHclBhp6cHx Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/01/2015 02:22 PM, Markus Armbruster wrote: > With the previous commit, the generated marshalers just work, and save > us a bit of handwritten code. >=20 Generated code grows, because you are now generating the hook :) qmp-commands.h | 6 ++ qmp-marshal.c | 144 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+) > Signed-off-by: Markus Armbruster > --- > include/monitor/monitor.h | 3 --- > qapi-schema.json | 9 +++------ > qmp-commands.hx | 6 +++--- > qmp.c | 20 +++++++------------- > scripts/qapi.py | 1 + > 5 files changed, 14 insertions(+), 25 deletions(-) >=20 > +++ b/qmp.c > @@ -229,11 +229,9 @@ ObjectPropertyInfoList *qmp_qom_list(const char *p= ath, Error **errp) > } > =20 > /* FIXME: teach qapi about how to pass through Visitors */ > -void qmp_qom_set(QDict *qdict, QObject **ret, Error **errp) > +void qmp_qom_set(const char *path, const char *property, QObject *valu= e, > + Error **errp) The FIXME seems stale now. > =20 > -void qmp_object_add(QDict *qdict, QObject **ret, Error **errp) > +void qmp_object_add(const char *type, const char *id, > + bool has_props, QObject *props, Error **errp) > { > - const char *type =3D qdict_get_str(qdict, "qom-type"); > - const char *id =3D qdict_get_str(qdict, "id"); > - QObject *props =3D qdict_get(qdict, "props"); > const QDict *pdict =3D NULL; > QmpInputVisitor *qiv; > =20 Continuing: if (props) { pdict =3D qobject_to_qdict(props); if (!pdict) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict"= ); return; } } I know that we guarantee that all pointers are initialized to NULL in our marshaler, so this is correct; but wouldn't it be more idiomatic to write 'if (has_props)' as the condition? (And it would make a nice followup project for someone to figure out how to get rid of have_FOO arguments for strings and objects where NULL is a nice witness; it is only integers and booleans that require them - but doing that will touch a lot of the tree, and is a series of its own) What I pointed out is minor, so you can fix it and add: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --TBhlJMkrg4lDCsgOjOaUUaLHclBhp6cHx 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/ iQEcBAEBCAAGBQJVsWkEAAoJEKeha0olJ0Nqbp0H/jqWvOH0QnpTUf4cmkduc/49 m0sT0UQJdwtBft242PI16RJ4e8ttVwlBjwj9axpzx9iLsALr2bMYoiVLrVpfM+ST bGMu+tInl27M1OtIvdGZIMbk0lHBCmMp0Omz4DHn6k6/xHBuR6B1ytlPaAvMRmHA QgkGVnqJkFQTYb4lVynO3dDbmHEHLRSwxwje0qeV4OtoXJ7UXUA+F7QCmqJZ+L9d 5inuUImGop6gtka/4wsQbkzXT+lqWZEo6hUeBxyDZWj+6yaKQ4TOF5wBsF0qQ4Bm 9zNdKGhJOLJpLSNIYkKlbOYw+kTJgjxakvbYg2X00u9Pn8TzOSm6A4PWw+NSvxo= =JNkV -----END PGP SIGNATURE----- --TBhlJMkrg4lDCsgOjOaUUaLHclBhp6cHx--