On Mon, 9 Dec 2019 15:02:38 +0100
Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
> On 12/9/19 2:28 PM, Greg Kurz wrote:
> > PPCVirtualHypervisor is an interface instance. It should never be
> > dereferenced. Drop the dummy type definition for extra safety, which
> > is the common practice with QOM interfaces.
>
> This "common practice" is also referenced in commit 00ed3da9b5:
>
> xics: Minor fixes for XICSFabric interface
>
> Interface instances should never be directly dereferenced. So, the
> common
> practice is to make them incomplete types to make sure no-one does
> that.
> XICSFrabric, however, had a dummy type which is less safe.
>
> We were also using OBJECT_CHECK() where we should have been using
> INTERFACE_CHECK().
>
> This indeed follow the changes from commit aa1b35b975d8:
>
> qom: make interface types abstract
>
> Interfaces don't have instance, let's make the interface type really
> abstract to avoid confusion.
>
> Now I can't find guidelines for this. If you don't know about it and use
> 'git-grep', it is very confusing to see we use structures we never define.
>
I agree that this deliberate usage of incomplete types isn't common.
> Can we document this use please?
>
Probably we could amend the related section in the object.h header file.
Something like:
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -200,8 +200,11 @@ typedef struct InterfaceInfo InterfaceInfo;
*
* Interfaces allow a limited form of multiple inheritance. Instances are
* similar to normal types except for the fact that are only defined by
- * their classes and never carry any state. You can dynamically cast an object
- * to one of its #Interface types and vice versa.
+ * their classes and never carry any state. As a consequence, a pointer to
+ * an interface instance should always be of incomplete type in order to be
+ * sure it cannot be dereferenced.
+ * You can dynamically cast an object to one of its #Interface types and vice
+ * versa.
*
* # Methods #
*
And even better, we could maybe come up with a way to detect that a type
was wrongly defined with coccinelle ?
> > Signed-off-by: Greg Kurz <gr...@kaod.org>
> > ---
> > target/ppc/cpu.h | 4 ----
> > 1 file changed, 4 deletions(-)
> >
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index e3e82327b723..ab7d07d66047 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -1220,10 +1220,6 @@ PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr);
> > PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr);
> > PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc);
> >
> > -struct PPCVirtualHypervisor {
> > - Object parent;
> > -};
> > -
> > struct PPCVirtualHypervisorClass {
> > InterfaceClass parent;
> > void (*hypercall)(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu);
> >
> >
>
