From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37750) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avndr-0001fR-7y for qemu-devel@nongnu.org; Thu, 28 Apr 2016 11:14:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avndl-0003Ai-DC for qemu-devel@nongnu.org; Thu, 28 Apr 2016 11:14:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38377) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avndl-0003AY-65 for qemu-devel@nongnu.org; Thu, 28 Apr 2016 11:14:05 -0400 References: <1461801715-24307-1-git-send-email-eblake@redhat.com> <1461801715-24307-10-git-send-email-eblake@redhat.com> <87wpnhkczg.fsf@dusky.pond.sub.org> From: Eric Blake Message-ID: <572228BB.7010403@redhat.com> Date: Thu, 28 Apr 2016 09:14:03 -0600 MIME-Version: 1.0 In-Reply-To: <87wpnhkczg.fsf@dusky.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="JKe5bGRCDdfpXEit3SF6Cts6HJm4uljVm" Subject: Re: [Qemu-devel] [PATCH v15 09/23] qom: Wrap prop visit in visit_start_struct List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, =?UTF-8?Q?Andreas_F=c3=a4rber?= This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --JKe5bGRCDdfpXEit3SF6Cts6HJm4uljVm Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 04/28/2016 08:46 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> The qmp-input visitor was allowing callers to play rather fast >> and loose: when visiting a QDict, you could grab members of the >> root dictionary without first pushing into the dict; the final >> such culprit was the QOM code for converting to and from object >> properties. But we are about to tighten the input visitor, at >> which point user_creatable_add_type() as called with a QMP input >> visitor via qmp_object_add() MUST follow the same paradigms as >> everyone else, of pushing into the struct before grabbing its >> keys. >> >> >> $ ./x86_64-softmmu/qemu-system-x86_64 -nographic -nodefaults -qmp stdi= o -object secret,id=3Dsec0,data=3Dletmein,format=3Draw,foo=3Dbar >> qemu-system-x86_64: Property '.foo' not found >=20 > Let's update the error message now that the error message regression is= > fixed. Can do on commit. I see when you fixed the error message regression your commits omitted data=3Dletmein,format=3Draw; I included them here because they are marked= mandatory in qapi, to make sure that we didn't have to think about whether excess arguments trump missing mandatory arguments in terms of error reporting. Up to you if you also feel comfortable shortening the command line to match the one in your commit message that fixed the regression. >> +++ b/qom/object_interfaces.c >> @@ -120,12 +120,20 @@ Object *user_creatable_add_type(const char *type= , const char *id, >> >> obj =3D object_new(type); >> if (qdict) { >> + visit_start_struct(v, NULL, NULL, 0, &local_err); >> + if (local_err) { >> + goto out; >> + } >> for (e =3D qdict_first(qdict); e; e =3D qdict_next(qdict, e))= { >> object_property_set(obj, v, e->key, &local_err); >> if (local_err) { >> - goto out; >> + break; >> } >> } >> + visit_end_struct(v, local_err ? NULL : &local_err); >> + if (local_err) { >> + goto out; >> + } >> } >> >> object_property_add_child(object_get_objects_root(), >=20 > Hmm. How should this behave if !qdict? Pre-existing. !qdict is not possible from qmp_object-add() (which throws QERR_INVALID_PARAMETER_TYPE on a missing or wrong QObject type for 'props= '). Also not possible from user_creatable_add(). We could turn it into assert(qdict) and drop a level of indentation. >=20 > However, both callers seem to pass non-null qdict. Should we sidestep > the question by making that a precondition? We came to the same conclusion, so yes :) --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --JKe5bGRCDdfpXEit3SF6Cts6HJm4uljVm 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/ iQEcBAEBCAAGBQJXIii7AAoJEKeha0olJ0NqwVgH/0y6QC23mPx0xlPWLNzSCTs1 FfJolqM21oTOKAfZffew44vomymIQf/L6QFC5dDOJamLVn2K7BL8RzVqH5tCzk5U gop4moDWTgbhJGzQscjmPfaHtgGCz4H5urormIQEdhXODeKLkhKcZ4CcXz0dh6+N YYJ0IUYtgPbnrJI0lJvhn6VF9B9pYAFv8XNRdBdd4JmiDhzDOqzaDgNoKmRzFWXP 2t7PDGpV7Lg2a/gTAYQJbCOJW8Fmdb/dA09IYs/j95y1QXNfjExQ0ydCycvlUTVe mYcTTCHEVSa7MPvfWfT5Su6aAvZAcXhAzLxY4n9lhxtFhZ15I5dX1tGnu9uJIJw= =lIHG -----END PGP SIGNATURE----- --JKe5bGRCDdfpXEit3SF6Cts6HJm4uljVm--