diff mbox series

[01/20] xics: Use incomplete type for XICSFabric

Message ID 20190925064534.19155-2-david@gibson.dropbear.id.au (mailing list archive)
State New, archived
Headers show
Series spapr: IRQ subsystem cleanups | expand

Commit Message

David Gibson Sept. 25, 2019, 6:45 a.m. UTC
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.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 include/hw/ppc/xics.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Cédric Le Goater Sept. 25, 2019, 6:55 a.m. UTC | #1
On 25/09/2019 08:45, David Gibson wrote:
> 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.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  include/hw/ppc/xics.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 64a2c8862a..1ae34ce9cd 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -147,9 +147,7 @@ struct ICSIRQState {
>      uint8_t flags;
>  };
>  
> -struct XICSFabric {
> -    Object parent;
> -};
> +typedef struct XICSFabric XICSFabric;
>  
>  #define TYPE_XICS_FABRIC "xics-fabric"
>  #define XICS_FABRIC(obj)                                     \
> 

you should also change :

#define XICS_FABRIC(obj)                                     \
    OBJECT_CHECK(XICSFabric, (obj), TYPE_XICS_FABRIC)


and use INTERFACE_CHECK()

C.
Greg Kurz Sept. 25, 2019, 7:45 a.m. UTC | #2
On Wed, 25 Sep 2019 16:45:15 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  include/hw/ppc/xics.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 64a2c8862a..1ae34ce9cd 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -147,9 +147,7 @@ struct ICSIRQState {
>      uint8_t flags;
>  };
>  
> -struct XICSFabric {
> -    Object parent;
> -};

Ok.

> +typedef struct XICSFabric XICSFabric;
>  

Nak. We already have this statement earlier in this file and
we must keep it there since it is used in the definition of
some other types. I guess this doesn't cause any compiler
warning because the type is incomplete.

With this duplicate typedef removed,

Reviewed-by: Greg Kurz <groug@kaod.org>

>  #define TYPE_XICS_FABRIC "xics-fabric"
>  #define XICS_FABRIC(obj)                                     \
Greg Kurz Sept. 25, 2019, 7:48 a.m. UTC | #3
On Wed, 25 Sep 2019 08:55:50 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> On 25/09/2019 08:45, David Gibson wrote:
> > 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.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  include/hw/ppc/xics.h | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> > index 64a2c8862a..1ae34ce9cd 100644
> > --- a/include/hw/ppc/xics.h
> > +++ b/include/hw/ppc/xics.h
> > @@ -147,9 +147,7 @@ struct ICSIRQState {
> >      uint8_t flags;
> >  };
> >  
> > -struct XICSFabric {
> > -    Object parent;
> > -};
> > +typedef struct XICSFabric XICSFabric;
> >  
> >  #define TYPE_XICS_FABRIC "xics-fabric"
> >  #define XICS_FABRIC(obj)                                     \
> > 
> 
> you should also change :
> 
> #define XICS_FABRIC(obj)                                     \
>     OBJECT_CHECK(XICSFabric, (obj), TYPE_XICS_FABRIC)
> 
> 
> and use INTERFACE_CHECK()
> 

Yes but I think this deserves a separate patch.

> C.
diff mbox series

Patch

diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 64a2c8862a..1ae34ce9cd 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -147,9 +147,7 @@  struct ICSIRQState {
     uint8_t flags;
 };
 
-struct XICSFabric {
-    Object parent;
-};
+typedef struct XICSFabric XICSFabric;
 
 #define TYPE_XICS_FABRIC "xics-fabric"
 #define XICS_FABRIC(obj)                                     \