>> Would it be valid to do something like this (skipping elements without a
>> proper visit_type_int)
>>
>> visit_start_list();
>> visit_next_list(); more input, returns "there's more"
>> visit_next_list(); parses "1-3,", buffers 2-3, skips over 1
>> visit_type_int(); returns 2
>> ...
>
> Excellent question!
>
> Here's the relevant part of visit_start_list()'s contract in visitor.h:
>
> * After visit_start_list() succeeds, the caller may visit its members
> * one after the other. A real visit (where @obj is non-NULL) uses
> * visit_next_list() for traversing the linked list, while a virtual
> * visit (where @obj is NULL) uses other means. For each list
> * element, call the appropriate visit_type_FOO() with name set to
> * NULL and obj set to the address of the value member of the list
> * element. Finally, visit_end_list() needs to be called with the
> * same @list to clean up, even if intermediate visits fail. See the
> * examples above.
>
> So, you *may* visit members, and you *must* call visit_end_list().
>
> But what are "real" and "virtual" visits? Again, the contract:
>
> * @list must be non-NULL for a real walk, in which case @size
> * determines how much memory an input or clone visitor will allocate
> * into *@list (at least sizeof(GenericList)). Some visitors also
> * allow @list to be NULL for a virtual walk, in which case @size is
> * ignored.
>
> I'm not sure whether I just decreased or increased global confusion ;)
I was reading over these comments and got slightly confused :)
>
> The file comment explains:
>
> * The QAPI schema defines both a set of C data types, and a QMP wire
> * format. QAPI objects can contain references to other QAPI objects,
> * resulting in a directed acyclic graph. QAPI also generates visitor
> * functions to walk these graphs. This file represents the interface
> * for doing work at each node of a QAPI graph; it can also be used
> * for a virtual walk, where there is no actual QAPI C struct.
>
> A real walk with an output visitor works like this (error handling
> omitted for now):
>
> Error *err = NULL;
> intList *tail;
> size_t size = sizeof(**obj);
>
> // fetch list's head into *obj, start the list in output
> visit_start_list(v, name, (GenericList **)obj, size, &err);
>
> // iterate over list's tails
> // below the hood, visit_next_list() iterates over the GenericList
> for (tail = *obj; tail;
> tail = (intList *)visit_next_list(v, (GenericList *)tail, size)) {
> // visit current tail's first member, add it to output
> visit_type_int(v, NULL, &tail->value, &err);
> }
>
> // end the list in output
> visit_end_list(v, (void **)obj);
>
> The exact same code works for a real walk with an input visitor, where
> visit_next_list() iterates over the *input* and builds up the
> GenericList. Compare qobject_input_next_list() and
> qobject_output_next_list().
>
> The code above is a straight copy of generated visit_type_intList() with
> error handling cut and comments added.
>
> A real walk works on a QAPI-generated C type. QAPI generates a real
> walk for each such type. Open-coding a real walk would senselessly
> duplicate the generated one, so we don't. Thus, a real walk always
> visits each member.
>
> Okay, I lied: it visits each member until it runs into an error and
> bails out. See generated visit_type_intList() in
> qapi/qapi-builtin-visit.c.
>
> A virtual walk doesn't work with a GenericList *. Calling
> visit_next_list() makes no sense then. visitor.h gives this example of
> a virtual walk:
Alright, so we must not support virtual walks. But supporting it would
not harm :)
>
> * Thus, a virtual walk corresponding to '{ "list": [1, 2] }' looks
> * like:
> *
> * <example>
> * Visitor *v;
> * Error *err = NULL;
> * int value;
> *
> * v = FOO_visitor_new(...);
> * visit_start_struct(v, NULL, NULL, 0, &err);
> * if (err) {
> * goto out;
> * }
> * visit_start_list(v, "list", NULL, 0, &err);
> * if (err) {
> * goto outobj;
> * }
> * value = 1;
> * visit_type_int(v, NULL, &value, &err);
> * if (err) {
> * goto outlist;
> * }
> * value = 2;
> * visit_type_int(v, NULL, &value, &err);
> * if (err) {
> * goto outlist;
> * }
> * outlist:
> * visit_end_list(v, NULL);
> * if (!err) {
> * visit_check_struct(v, &err);
> * }
> * outobj:
> * visit_end_struct(v, NULL);
> * out:
> * error_propagate(errp, err);
> * visit_free(v);
>
> Why could this be useful?
>
> 1. With the QObject input visitor, it's an alternative way to
> destructure a QObject into a bunch of C variables. The "obvious" way
> would be calling the QObject accessors. By using the visitors you
> get much the error checking code for free. YMMV.
>
> 2. With the QObject output visitor, it's an alternative way to build up
> a QObject. The "obvious" way would be calling the constructors
> directly.
>
> 3. With the string input / output visitors, it's a way to parse / format
> string visitor syntax without having to construct the C type first.
>
> Right now, we have no virtual list walks outside tests. We do have
> virtual struct walks outside tests.
>
>> Or mixing types
>>
>> visit_start_list();
>> visit_next_list();
>> visit_type_int64();
>> visit_next_list();
>> visit_type_uint64();
>
> Another excellent question.
>
> QAPI can only express homogeneous lists, i.e. lists of some type T.
>
> The generated visit_type_TList call the same visit_type_T() for all list
> members.
Okay, my point would be: Somebody coding its own visit code should not
assume this to work.
>
> QAPI type 'any' is the top type, but visit_type_anyList() still calls
> visit_type_any() for all list members.
>
> Virtual walks could of course do anything. Since they don't exist
> outside tests, we can outlaw doing things that cause us trouble.
>
> The virtual walks we currently have in tests/ seem to walk only
> homogeneous lists, i.e. always call the same visit_type_T().
Okay, so bailing out if types are switched (e.g. just about to report a
range of uin64_t and somebody asks for a int64_t) is valid.
>
>> IOW, can I assume that after every visit_next_list(), visit_type_X is
>> called, and that X remains the same for one pass over the list?
>
> As far as I can tell, existing code is just fine with that assumption.
> We'd have to write it into the contract, though.
>
>> Also, I assume it is supposed to work like this
>>
>> visit_start_list();
>> visit_next_list(); more input, returns "there's more"
>> visit_type_int(); returns 1 (parses 1-3, buffers 1-3)
>> visit_type_int(); returns 1
>> visit_type_int(); returns 1
>> visit_next_list(); more input, unbuffers 1
>> visit_type_int(); returns 2
>>
>> So unbuffering actually happens on visit_next_list()?
>
> I believe this violates the contract.
>
> If it's a real walk, the contract wants you to call visit_next_list()
> between subsequent visit_type_int().
>
> If it's a virtual walk, calling visit_next_list() makes no sense.
>
> More questions?
>
Thanks for the excessive answer! I think that should be enough to come
up with a RFC of a *rewrite* of the string input visitor :)
--
Thanks,
David / dhildenb
