On Fri, Aug 21, 2020 at 11:47:32AM +1000, David Gibson wrote:
> On Thu, Aug 20, 2020 at 05:55:29PM -0400, Eduardo Habkost wrote:
> > While trying to convert TypeInfo declarations to the new
> > OBJECT_DECLARE* macros, I've stumbled on a few suspicious cases
> > where instance_size or class_size is not set, despite having type
> > checker macros that use a specific type.
> >
> > The ones with "WARNING" are abstract types (maybe not serious if
> > subclasses set the appropriate sizes). The ones with "ERROR"
> > don't seem to be abstract types.
>
>
> Comment on the ones within my area:
> >
> > WARNING: hw/input/adb.c:310:1: class_size should be set to
> > sizeof(ADBDeviceClass)?
>
> Yeah, that looks like a bug (though we'll get away with it because
> it's abstract).
Right, luckily we are not touching any ADBDeviceClass field
inside adb_device_class_init().
>
> > WARNING: hw/ppc/pnv_lpc.c:771:1: instance_size should be set to
> > sizeof(PnvLpcController)?
>
> Ditto.
Agreed.
>
> Should I make fixes for these, or will you?
Please send the fixes, and I will apply them before running the
TypeInfo conversion script.
>
> > ERROR: hw/ppc/spapr_drc.c:771:1: instance_size should be set to
> > sizeof(SpaprDrc)?
>
> I'm confused by this one. I'm not exactly sure which definition is
> tripping the error, and AFAICT they should all be correctly inheriting
> instance_size from either TYPE_SPAPR_DR_CONNECTOR or
> TYPE_SPAPR_DRC_PHSYICAL. If anything, it looks like
> TYPE_SPAPR_DRC_PHB could drop it's explicit override of instance_size.
The error is triggered because of this type checking macro at
include/hw/ppc/spapr_drc.h:
#define SPAPR_DRC_PCI(obj) OBJECT_CHECK(SpaprDrc, (obj), \
TYPE_SPAPR_DRC_PCI)
The expectation is that whatever type you use in OBJECT_CHECK
will be the one used for instance_size. The script also looks at
the parent type, to reduce false positives, but this case was
flagged because SPAPR_DRC_PCI uses SpaprDrc, but the parent type
(SPAPR_DRC_PHYSICAL) uses SpaprDrcPhysical.
Now, I don't understand why we have so many instance checker
macros that use the same typedef (SpaprDrc). If the code needs a
valid SpaprDrc pointer, it can just use SPAPR_DR_CONNECTOR().
--
Eduardo
