[Qemu-devel] QAPI and empty structs

[Qemu-devel] QAPI and empty structs

[Peter Maydell]

In qapi-schema.json we have a couple of entries that define
empty structures, like this:

{ 'type': 'ChardevDummy', 'data': { } }

In the generated qapi-types.h these are turned into empty C structs:

struct ChardevDummy
{
};

and clang warns about them:
./qapi-types.h:3752:1: warning: empty struct has size 0 in C, size 1
in C++ [-Wextern-c-compat]
struct Abort
^
./qapi-types.h:3811:1: warning: empty struct has size 0 in C, size 1
in C++ [-Wextern-c-compat]
struct NetdevNoneOptions
^
./qapi-types.h:4282:1: warning: empty struct has size 0 in C, size 1
in C++ [-Wextern-c-compat]
struct ChardevDummy
^

Although you could argue that we don't care about differences
in C and C++ semantics, having a zero-sized struct floating
around seems a bit risky to me, since for instance a naive
attempt to g_malloc() space for it will return NULL.

How should we handle these? Should the qapi-types generator
stick a dummy field in so they aren't zero sized? Are they
actually an error in the schema? (what's the point of them?)
Do we claim that actually all our code correctly handles
zero sized structs and just suppress the clang warning?

thanks
-- PMM

Re: [Qemu-devel] QAPI and empty structs

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

On 12/23/2014 08:50 AM, Peter Maydell wrote:
> In qapi-schema.json we have a couple of entries that define
> empty structures, like this:
> 
> { 'type': 'ChardevDummy', 'data': { } }
> 
> In the generated qapi-types.h these are turned into empty C structs:
> 

> Although you could argue that we don't care about differences
> in C and C++ semantics, having a zero-sized struct floating
> around seems a bit risky to me, since for instance a naive
> attempt to g_malloc() space for it will return NULL.

Oh, good point.

> 
> How should we handle these? Should the qapi-types generator
> stick a dummy field in so they aren't zero sized?

Sounds like the best plan to me.

> Are they
> actually an error in the schema? (what's the point of them?)

No, they are valid - it is how you express a union between a branch that
needs no additional members compared to any other branch.

> Do we claim that actually all our code correctly handles
> zero sized structs and just suppress the clang warning?

I'd be fine with adding a dummy member.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] QAPI and empty structs

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 12/23/2014 08:50 AM, Peter Maydell wrote:
>> In qapi-schema.json we have a couple of entries that define
>> empty structures, like this:
>> 
>> { 'type': 'ChardevDummy', 'data': { } }
>> 
>> In the generated qapi-types.h these are turned into empty C structs:
>> 
>
>> Although you could argue that we don't care about differences
>> in C and C++ semantics, having a zero-sized struct floating
>> around seems a bit risky to me, since for instance a naive
>> attempt to g_malloc() space for it will return NULL.
>
> Oh, good point.

Less of an issue than with malloc(), because NULL isn't both an error
and a success value.

>> How should we handle these? Should the qapi-types generator
>> stick a dummy field in so they aren't zero sized?
>
> Sounds like the best plan to me.

I wouldn't bother myself, but if somebody posted working patches, I
wouldn't bother objecting to the idea, either.

>> Are they
>> actually an error in the schema? (what's the point of them?)
>
> No, they are valid - it is how you express a union between a branch that
> needs no additional members compared to any other branch.

Yes.

>> Do we claim that actually all our code correctly handles
>> zero sized structs and just suppress the clang warning?
>
> I'd be fine with adding a dummy member.

The clang warning is for code meant to be compiled both as C and as C++,
i.e. it's of no interest to us.  Suppressing it sounds fine to me.

Re: [Qemu-devel] QAPI and empty structs

[Peter Maydell]

On 12 January 2015 at 15:31, Markus Armbruster <armbru@redhat.com> wrote:
> Eric Blake <eblake@redhat.com> writes:
>> On 12/23/2014 08:50 AM, Peter Maydell wrote:
>>> Although you could argue that we don't care about differences
>>> in C and C++ semantics, having a zero-sized struct floating
>>> around seems a bit risky to me, since for instance a naive
>>> attempt to g_malloc() space for it will return NULL.
>>
>> Oh, good point.
>
> Less of an issue than with malloc(), because NULL isn't both an error
> and a success value.

No, it's just a value that pretty much no caller is going
to expect to get and which they're very likely to be buggy
in dealing with...

>>> How should we handle these? Should the qapi-types generator
>>> stick a dummy field in so they aren't zero sized?
>>
>> Sounds like the best plan to me.
>
> I wouldn't bother myself, but if somebody posted working patches, I
> wouldn't bother objecting to the idea, either.

http://patchwork.ozlabs.org/patch/423777/  :-)

-- PMM