On 15 February 2013 13:45, Andreas Färber <afaer...@suse.de> wrote: > Am 15.02.2013 14:38, schrieb Peter Maydell: >> On 15 February 2013 13:11, Andreas Färber <afaer...@suse.de> wrote: >>> Am 15.02.2013 12:45, schrieb Peter Maydell: >>>> --- a/hw/musicpal.c >>>> +++ b/hw/musicpal.c >>>> @@ -1031,6 +1031,21 @@ static const TypeInfo mv88w8618_flashcfg_info = { >>>> >>>> #define MP_BOARD_REVISION 0x31 >>>> >>>> +typedef struct { >>> >>> Anonymous struct >> >> No, it's a typedef. Why would you want to name the struct >> particularly when it's an error to then use that name rather >> than the typedef? Better to let the compiler make uses of >> 'struct Foo' rather than 'Foo' a compile failure. > > I'm pretty sure it has been requested by Blue and Anthony in the past... > Not sure if it makes a difference for gdb or something.
I've cc'd them. But unless somebody has an actual good reason for giving the struct in the typedef a completely unnecessary name I think leaving it unnamed is better. (This is distinct from genuinely anonymous structs with no 'struct foo' name or typedef name, which we do have a few of in the codebase. I agree those are better avoided.) >>>> +typedef SysBusDeviceClass MusicPalMiscClass; >>> >>> Please don't. Either define a struct and use it for .class_size or drop >>> the typedef. >> >> So my rationale here was "all classes should have a FooClass". >> If you have classes which don't have a FooClass then if at >> some later point you need to introduce a class struct you >> have to go round and locate all the places you wrote >> ParentClass and now need to change it to FooClass. If >> we always have a typedef everywhere then there is never >> a problem. >> >> More generally, I think we should prefer to avoid the kind of >> shortcut that the C implementation allows us to take. If we >> define a QOM class then that should mean you get the full range >> of expected things (a TYPE_FOO, a FOO macro, a FOO_CLASS and >> a FooClass type). >> >> If you prefer we could standardize on >> typedef struct { >> ParentClass parent; >> } FooClass; >> >> rather than typedef ParentClass FooClass; > It may need rework either way. Because aliasing via typedef gives > FooClass fields it will loose once it is turned into a real QOM class. > We had such an issue with i440fx in my PHB series, that's why I'm > sensitive to it. ;) OK, so that seems like an argument for always defining the empty-except-for-the-parentclass class struct, or does that run into problems too? -- PMM