[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v15 09/23] qom: Wrap prop visit in visit_start_s
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v15 09/23] qom: Wrap prop visit in visit_start_struct |
Date: |
Thu, 28 Apr 2016 09:14:03 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 04/28/2016 08:46 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> 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 stdio
>> -object secret,id=sec0,data=letmein,format=raw,foo=bar
>> qemu-system-x86_64: Property '.foo' not found
>
> 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=letmein,format=raw; 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 = object_new(type);
>> if (qdict) {
>> + visit_start_struct(v, NULL, NULL, 0, &local_err);
>> + if (local_err) {
>> + goto out;
>> + }
>> for (e = qdict_first(qdict); e; e = 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(),
>
> 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.
>
> 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 :)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v15 00/23] qapi visitor cleanups (post-introspection cleanups subset E), Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 07/23] qapi-commands: Wrap argument visit in visit_start_struct, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 03/23] qmp: Drop dead command->type, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 06/23] qmp-input: Don't consume input when checking has_member, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 09/23] qom: Wrap prop visit in visit_start_struct, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 04/23] qmp-input: Clean up stack handling, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 11/23] qmp-input: Refactor when list is advanced, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 13/23] tests: Add check-qnull, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 02/23] qapi: Guarantee NULL obj on input visitor callback error, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 02A/23] fixup! qapi: Guarantee NULL obj on input visitor callback error, Eric Blake, 2016/04/28
- [Qemu-devel] [PATCH v15 14/23] qapi: Add visit_type_null() visitor, Eric Blake, 2016/04/27