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.
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...
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'll study the rest of your reply next.
