Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Jan 09, 2019 at 06:13:01PM +0100, Cédric Le Goater wrote:
> On 1/9/19 5:39 PM, Thomas Huth wrote:
> > When compiling with Clang and -std=gnu99, I get the following errors:
> > 
> >   CC      ppc64-softmmu/hw/intc/xics_spapr.o
> > In file included from hw/intc/xics_spapr.c:34:
> > include/hw/ppc/xics.h:46:25: error: redefinition of typedef 'ICSState' is a C11 feature
> >       [-Werror,-Wtypedef-redefinition]
> > typedef struct ICSState ICSState;
> >                         ^
> > include/hw/ppc/spapr.h:18:25: note: previous definition is here
> > typedef struct ICSState ICSState;
> >                         ^
> > In file included from hw/intc/xics_spapr.c:34:
> > include/hw/ppc/xics.h:203:34: error: redefinition of typedef 'sPAPRMachineState' is a C11 feature
> >       [-Werror,-Wtypedef-redefinition]
> > typedef struct sPAPRMachineState sPAPRMachineState;
> >                                  ^
> > include/hw/ppc/spapr_irq.h:25:34: note: previous definition is here
> > typedef struct sPAPRMachineState sPAPRMachineState;
> >                                  ^
> >   CC      ppc64-softmmu/hw/intc/spapr_xive.o
> > In file included from hw/intc/spapr_xive.c:19:
> > include/hw/ppc/spapr_xive.h:38:3: error: redefinition of typedef 'sPAPRXive' is a C11 feature
> >       [-Werror,-Wtypedef-redefinition]
> > } sPAPRXive;
> >   ^
> > include/hw/ppc/spapr.h:20:26: note: previous definition is here
> > typedef struct sPAPRXive sPAPRXive;
> >                          ^
> > In file included from hw/intc/spapr_xive.c:19:
> > include/hw/ppc/spapr_xive.h:45:34: error: redefinition of typedef 'sPAPRMachineState' is a C11 feature
> >       [-Werror,-Wtypedef-redefinition]
> > typedef struct sPAPRMachineState sPAPRMachineState;
> >                                  ^
> > include/hw/ppc/spapr_irq.h:25:34: note: previous definition is here
> > typedef struct sPAPRMachineState sPAPRMachineState;
> >                                  ^
> >   CC      ppc64-softmmu/hw/char/spapr_vty.o
> > In file included from hw/char/spapr_vty.c:8:
> > In file included from include/hw/ppc/spapr.h:12:
> > include/hw/ppc/spapr_xive.h:45:34: error: redefinition of typedef 'sPAPRMachineState' is a C11 feature
> >       [-Werror,-Wtypedef-redefinition]
> > typedef struct sPAPRMachineState sPAPRMachineState;
> >                                  ^
> > include/hw/ppc/spapr_irq.h:25:34: note: previous definition is here
> > typedef struct sPAPRMachineState sPAPRMachineState;
> >                                  ^
> > 
> > Since we're going to make -std=gnu99 mandatory, fix these issues
> > by including the right header files indead of typedeffing stuff twice.
> >
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >  include/hw/ppc/spapr.h      | 4 ++--
> >  include/hw/ppc/spapr_xive.h | 2 +-
> >  include/hw/ppc/xics.h       | 4 ++--
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 2c77a8b..6a5ae4f 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -8,6 +8,8 @@
> >  #include "hw/mem/pc-dimm.h"
> >  #include "hw/ppc/spapr_ovec.h"
> >  #include "hw/ppc/spapr_irq.h"
> > +#include "hw/ppc/xics.h"
> > +#include "hw/ppc/spapr_xive.h"
> >  
> >  struct VIOsPAPRBus;
> >  struct sPAPRPHBState;
> > @@ -15,8 +17,6 @@ struct sPAPRNVRAM;
> >  typedef struct sPAPREventLogEntry sPAPREventLogEntry;
> >  typedef struct sPAPREventSource sPAPREventSource;
> >  typedef struct sPAPRPendingHPT sPAPRPendingHPT;
> > -typedef struct ICSState ICSState;
> > -typedef struct sPAPRXive sPAPRXive;
> >  
> >  #define HPTE64_V_HPTE_DIRTY     0x0000000000000040ULL
> >  #define SPAPR_ENTRY_POINT       0x100
> > diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> > index 728735d..aff4366 100644
> > --- a/include/hw/ppc/spapr_xive.h
> > +++ b/include/hw/ppc/spapr_xive.h
> > @@ -42,7 +42,7 @@ bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
> >  void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
> >  qemu_irq spapr_xive_qirq(sPAPRXive *xive, uint32_t lisn);
> >  
> > -typedef struct sPAPRMachineState sPAPRMachineState;
> > +#include "hw/ppc/spapr.h"  /* for sPAPRMachineState */
> 
> so both files include each other, how nice ...

If the header files are mutually dependent it makes me wonder what the
point of having them split up is ?

Feels like either they need to be merged, or they need to be split up
and refactored even more to remove the mutual dependancy.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|