Am 18.02.2021 um 14:39 hat Markus Armbruster geschrieben:
> Kevin Wolf <kw...@redhat.com> writes:
>
> > Am 17.02.2021 um 16:32 hat Markus Armbruster geschrieben:
> >> Kevin Wolf <kw...@redhat.com> writes:
> >>
> >> > When looking for an object in a struct in the external representation,
> >> > check not only the currently visited struct, but also whether an alias
> >> > in the current StackObject matches and try to fetch the value from the
> >> > alias then. Providing two values for the same object through different
> >> > aliases is an error.
> >> >
> >> > Signed-off-by: Kevin Wolf <kw...@redhat.com>
> >>
> >> Looking just at qobject_input_try_get_object() for now.
> >
> > :-(
> >
> > This patch doesn't even feel that complicated to me.
>
> I suspect it's just me having an unusually obtuse week.
>
> The code is straightforward enough. What I'm missing is a bit of "how
> does this accomplish the goal" and "why is this safe" here and there.
>
> > Old: Get the value from the QDict of the current StackObject with the
> > given name.
> >
> > New: First do alias resolution (with find_object_member), which results
> > in a StackObject and a name, and that's the QDict and key where you get
> > the value from.
>
> This part I understand.
>
> We simultaneously walk the QAPI type and the input QObject, consuming
> input as we go.
>
> Whenever the walk leaves a QAPI object type, we check the corresponding
> QObject has been consumed completely.
>
> With aliases, we additionally look for input in a certain enclosing
> input object (i.e. up the recursion stack). If found, consume.
>
> > Minor complication: Aliases can refer to members of nested objects that
> > may not be provided in the input. But we want these to work.
> >
> > For example, my chardev series defines aliases to flatten
> > SocketAddressLegacy (old syntax, I haven't rebased it yet):
> >
> > { 'union': 'SocketAddressLegacy',
> > 'data': {
> > 'inet': 'InetSocketAddress',
> > 'unix': 'UnixSocketAddress',
> > 'vsock': 'VsockSocketAddress',
> > 'fd': 'String' },
> > 'aliases': [
> > {'source': ['data']},
> > {'alias': 'fd', 'source': ['data', 'str']}
> > ]}
> >
> > Of course, the idea is that this input should work:
> >
> > { 'type': 'inet', 'hostname': 'localhost', 'port': '1234' }
> >
> > However, without implicit objects, parsing 'data' fails because it
> > expects an object, but none is given (we specified all of its members on
> > the top level through aliases). What we would have to give is:
> >
> > { 'type': 'inet', 'hostname': 'localhost', 'port': '1234', 'data': {} }
> >
> > And _that_ would work. Visiting 'data' succeeds because we now have the
> > object that the visitor expects, and when visiting its members, the
> > aliases fill in all of the mandatory values.
> >
> > So what this patch does is to implicitly assume the 'data': {} instead
> > of erroring out when we know that aliases exist that can still provide
> > values for the content of 'data'.
>
> Aliases exist than can still provide, but will they? What if input is
>
> {"type": "inet"}
>
> ?
>
> Your explanation makes me guess this is equivalent to
>
> {"type": "inet", "data": {}}
>
> which fails the visit, because mandatory members of "data" are missing.
> Fine.
Okay, if you want the gory details, then the answer is yes for this
case, but it depends.
If we're aliasing a single member, then we can easily check whether the
alias is actually specified. If it's not in the input, no implicit
object.
But in our example, it is a wildcard alias and we don't know yet which
aliases it will use. This depends on what the visitor for the implicit
object will do (future tense).
> If we make the members optional, it succeeds: qobject_input_optional()
> checks both the regular and the aliased input, finds neither, and
> returns false. Still fine.
>
> What if "data" is optional, too? Hmmm...
Yes, don't use optional objects in the middle of the path of a wildcard
alias unless there is no semantic difference between empty object and
absent object. This is documented in the code, but it might actually
still be missing from qapi-code-gen.txt.
> Example:
>
> { 'struct': 'Outer',
> 'data': { '*inner': 'Inner' },
>
> { 'struct': 'Inner',
> 'data': { '*true-name': 'str' } }
>
> For input {}, we get an Outer object with
>
> .has_inner = false,
> .inner = NULL
>
> Now add
>
> 'aliases': [ { 'name': 'alias-name',
> 'source': [ 'inner', 'true-name' ] } ] }
>
> What do we get now? Is it
>
> .has_inner = true,
> .inner = { .has_true_name = false,
> .true_name = NULL }
>
> perhaps?
I think this is the result you would get if you had used a wildcard
alias. But since you used a single-member alias, we would see that
'alias-name' is not in the input and actually still return the original
result:
.has_inner = false,
.inner = NULL
Kevin
