mdroth <mdr...@linux.vnet.ibm.com> writes:
> On Tue, Feb 12, 2013 at 05:56:00PM +0100, Markus Armbruster wrote:
>> Gerd Hoffmann <kra...@redhat.com> writes:
>>
>> > Hi,
>> >
>> >> But why nested discriminators?
>> >>
>> >> regular files: type=file
>> >> serial : type=port, data.type=serial
>> >> parallel : type=port, data.type=parallel
>> >>
>> >> Simpler, and closer to existing -chardev:
>> >>
>> >> regular files: type=file
>> >> serial : type=serial
>> >> parallel : type=parallel
>> >
>> > Matter of taste IMHO.
>> > I can live with that too.
>> > Feel free to send patches.
>> >
>> >> I also dislike the pointless '"data" : {}' required by type pty and
>> >> null, but I can't figure out how to express 'void' in the schema.
>> >
>> > Someone mentioned it is possible to leave out empty data altogether.
>> > Didn't try whenever our marshaller actually accepts that though.
>>
>> Looks like it doesn't :(
>>
>> Empty objects work fine here:
>>
>> { 'type': 'ChardevDummy', 'data': { } }
>>
>> Generates the obvious
>>
>> struct ChardevDummy
>> {
>> };
>>
>> They don't work here:
>>
>> { 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
>> 'hostdev': 'ChardevHostdev',
>> 'socket' : 'ChardevSocket',
>> 'pty' : 'ChardevDummy',
>> 'null' : {} } }
>>
>> Generates
>>
>> struct ChardevBackend
>> {
>> ChardevBackendKind kind;
>> union {
>> void *data;
>> ChardevFile * file;
>> ChardevHostdev * hostdev;
>> ChardevSocket * socket;
>> ChardevDummy * pty;
>> void null;
>> };
>> };
>>
>> which is kind of cute, but the compiler doesn't like it.
>>
>> Anthony, Mike, is this a bug?
>
> Not exactly, but it's a relic that doesn't seem to be needed anymore.
> The code that does this is in scripts/qapi.py:
>
> def c_type(name):
> ...
> elif name == None or len(name) == 0:
> return 'void'
> ...
> else:
> return '%s *' % name
>
> The 'name' param being the value/type of a particular param/key in a
> QAPI dictionary that defines a schema-defined type.
>
> The reason '{}' maps to 'void' is so that in qapi-commands.py, where we
> generate
> stuff like the function signatures for qmp commands, we'd map something like:
>
> { 'command': 'my_cmd',
> 'data': { 'param1': 'Foo' },
> 'returns': {} }
>
> to:
>
> void my_cmd(Foo *param1);
>
> At some point in development we rightly decided that:
>
> { 'command': 'my_cmd',
> 'data': { 'param1': 'Foo' } }
>
> was more efficient, which triggers the 'name == None' case and has the same
> effect.
>
> So, as long as we stay consistent about defining commands in this fashion, we
> can map 'param_name': {} to something like 'struct {}', and use it in place of
> schema-defined dummy types.
>
> Though, as I mentioned on IRC, I think just defining a:
>
> { 'type': 'Dummy', 'data' {} }
>
> Somewhere in the schema and re-using that might actually be cleaner and have
> the same affect.
Yes. I don't think we really ought to support inline structures. It
keeps the declarations easier to read to make all structs top level
types.
Regards,
Anthony Liguori
