Eduardo Habkost <[email protected]> writes: > 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.
I think the pattern exists, but we deviate from it in quite a few places, probably just because it's so much boilerplate. Related: Daniel's "[PATCH 0/4] qom: reduce boilerplate required for declaring and defining objects". Perhaps Daniel has an opinion on taking shortcuts with 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. Rule of thumb: hw/ide/ is a bad example. I don't mean to belittle the efforts of quite a few people over the years. It used to be worse. >> 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. I'm fine with having a separate convention for leaf types if that helps, but please let's have a convention. I like my QOM boilerplate uncreative. >> 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 >>
