On Wed, Aug 05, 2020 at 09:41:25PM +0100, Peter Maydell wrote: > On Wed, 5 Aug 2020 at 20:49, Eduardo Habkost <[email protected]> wrote: > > > > The struct had a single field (IDEDevice dev), and is only used > > in the QOM type declarations and property lists. We can simply > > use the IDEDevice struct directly instead. > > > > Signed-off-by: Eduardo Habkost <[email protected]> > > @@ -327,7 +323,6 @@ static void ide_hd_class_init(ObjectClass *klass, void > > *data) > > static const TypeInfo ide_hd_info = { > > .name = "ide-hd", > > .parent = TYPE_IDE_DEVICE, > > - .instance_size = sizeof(IDEDrive), > > .class_init = ide_hd_class_init, > > }; > > This is one of those areas where this change works and reduces > amount of code, but on the other hand it means the QOM type > doesn't follow the common pattern for a leaf type of: > * it has a struct > * it has cast macros that cast to that struct > * the typeinfo instance_size is the size of that struct > (it wasn't exactly following this pattern before, of course).
Is this really a pattern that exists and we want to follow? I don't see why that pattern would be useful for simple leaf types. Also, in this case the code wasn't even following that pattern: it was using the same IDEDrive struct for all TYPE_IDE_DEVICE subtypes. > > We define in https://wiki.qemu.org/Documentation/QOMConventions > (in the 'When to create class types and macros' bit at the bottom) > what we expect for whether to provide class cast macros/a > class struct/class_size in the TypeInfo, essentially recommending > that types follow one of two patterns (simple leaf class with no > methods or class members, vs everything else) even if in a > particular case you could take a short-cut and not define > everything. We haven't really defined similar "this is the > standard pattern, provide it all even if you don't strictly > need it" rules for the instance struct/macros. Maybe we should? I think we should include the instance struct/macros in the recommendations there, but I would expect those recommendations to apply only to non-leaf types. > > Just a thought, not a nak; I know we have quite a number > of types that take this kind of "we don't really need to > provide all the standard QOM macros/structs/etc" approach > (some of which I wrote!). > > thanks > -- PMM > -- Eduardo
