* [Qemu-devel] [PATCH v3 0/2] Force the C standard to gnu99 @ 2019-01-09 16:39 Thomas Huth 2019-01-09 16:39 ` [Qemu-devel] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 Thomas Huth 2019-01-09 16:39 ` [Qemu-devel] [PATCH v3 2/2] configure: Force the C standard to gnu99 Thomas Huth 0 siblings, 2 replies; 19+ messages in thread From: Thomas Huth @ 2019-01-09 16:39 UTC (permalink / raw) To: qemu-devel Cc: Richard Henderson, Daniel P. Berrangé, pbonzini, peter.maydell, Markus Armbruster, qemu-ppc Different versions of GCC and Clang use different versions of the C standard by default. To avoid compilation problems with different compilers in the future, we should enforce one language level for all compilers. Since "gnu99" is the only usable option right now (later versions are still marked as experimental in GCC v4.8), we enforce compiling with -std=gnu99 now (in the second patch). This caused some new warnings/erros to appear with clang, which are fixed by the first patch now. Thomas Huth (2): include/hw/ppc: Fix compilation with clang -std=gnu99 configure: Force the C standard to gnu99 configure | 5 ++++- include/hw/ppc/spapr.h | 4 ++-- include/hw/ppc/spapr_xive.h | 2 +- include/hw/ppc/xics.h | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 16:39 [Qemu-devel] [PATCH v3 0/2] Force the C standard to gnu99 Thomas Huth @ 2019-01-09 16:39 ` Thomas Huth 2019-01-09 17:13 ` [Qemu-devel] [Qemu-ppc] " Cédric Le Goater 2019-01-09 17:46 ` Cédric Le Goater 2019-01-09 16:39 ` [Qemu-devel] [PATCH v3 2/2] configure: Force the C standard to gnu99 Thomas Huth 1 sibling, 2 replies; 19+ messages in thread From: Thomas Huth @ 2019-01-09 16:39 UTC (permalink / raw) To: qemu-devel Cc: Richard Henderson, Daniel P. Berrangé, pbonzini, peter.maydell, Markus Armbruster, qemu-ppc 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 */ void spapr_xive_hcall_init(sPAPRMachineState *spapr); void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt, diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h index 14afda1..a7f49a4 100644 --- a/include/hw/ppc/xics.h +++ b/include/hw/ppc/xics.h @@ -47,6 +47,8 @@ typedef struct ICSState ICSState; typedef struct ICSIRQState ICSIRQState; typedef struct XICSFabric XICSFabric; +#include "hw/ppc/spapr.h" + #define TYPE_ICP "icp" #define ICP(obj) OBJECT_CHECK(ICPState, (obj), TYPE_ICP) @@ -200,8 +202,6 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon); void ics_resend(ICSState *ics); void icp_resend(ICPState *ss); -typedef struct sPAPRMachineState sPAPRMachineState; - void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt, uint32_t phandle); int xics_kvm_init(sPAPRMachineState *spapr, Error **errp); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 16:39 ` [Qemu-devel] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 Thomas Huth @ 2019-01-09 17:13 ` Cédric Le Goater 2019-01-09 17:28 ` Daniel P. Berrangé 2019-01-09 17:46 ` Cédric Le Goater 1 sibling, 1 reply; 19+ messages in thread From: Cédric Le Goater @ 2019-01-09 17:13 UTC (permalink / raw) To: Thomas Huth, qemu-devel Cc: peter.maydell, Daniel P. Berrangé, Markus Armbruster, qemu-ppc, pbonzini, Richard Henderson 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 ... > void spapr_xive_hcall_init(sPAPRMachineState *spapr); > void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt, > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index 14afda1..a7f49a4 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -47,6 +47,8 @@ typedef struct ICSState ICSState; > typedef struct ICSIRQState ICSIRQState; > typedef struct XICSFabric XICSFabric; > > +#include "hw/ppc/spapr.h" > + and again ... I *really really* prefer forward declarations. Reviewed-by: Cédric Le Goater <clg@kaod.org> Grumbles, C. > #define TYPE_ICP "icp" > #define ICP(obj) OBJECT_CHECK(ICPState, (obj), TYPE_ICP) > > @@ -200,8 +202,6 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon); > void ics_resend(ICSState *ics); > void icp_resend(ICPState *ss); > > -typedef struct sPAPRMachineState sPAPRMachineState; > - > void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt, > uint32_t phandle); > int xics_kvm_init(sPAPRMachineState *spapr, Error **errp); > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 17:13 ` [Qemu-devel] [Qemu-ppc] " Cédric Le Goater @ 2019-01-09 17:28 ` Daniel P. Berrangé 2019-01-09 17:40 ` Cédric Le Goater 2019-01-09 21:20 ` Paolo Bonzini 0 siblings, 2 replies; 19+ messages in thread From: Daniel P. Berrangé @ 2019-01-09 17:28 UTC (permalink / raw) To: Cédric Le Goater Cc: Thomas Huth, qemu-devel, peter.maydell, Markus Armbruster, qemu-ppc, pbonzini, Richard Henderson 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 :| ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 17:28 ` Daniel P. Berrangé @ 2019-01-09 17:40 ` Cédric Le Goater 2019-01-09 17:47 ` Greg Kurz 2019-01-09 21:20 ` Paolo Bonzini 1 sibling, 1 reply; 19+ messages in thread From: Cédric Le Goater @ 2019-01-09 17:40 UTC (permalink / raw) To: Daniel P. Berrangé Cc: Thomas Huth, qemu-devel, peter.maydell, Markus Armbruster, qemu-ppc, pbonzini, Richard Henderson On 1/9/19 6:28 PM, Daniel P. Berrangé wrote: > 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 ? The XICS interrupt controller models are used in two different machines, sPAPR and PowerNV. > Feels like either they need to be merged, or they need to be split up > and refactored even more to remove the mutual dependancy. yes or that some spapr_* definitions do not belong to the common xics.h file. Thanks, C. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 17:40 ` Cédric Le Goater @ 2019-01-09 17:47 ` Greg Kurz 2019-01-10 6:47 ` Thomas Huth 0 siblings, 1 reply; 19+ messages in thread From: Greg Kurz @ 2019-01-09 17:47 UTC (permalink / raw) To: Cédric Le Goater Cc: Daniel P. Berrangé, peter.maydell, Thomas Huth, qemu-devel, Markus Armbruster, qemu-ppc, pbonzini, Richard Henderson On Wed, 9 Jan 2019 18:40:48 +0100 Cédric Le Goater <clg@kaod.org> wrote: > On 1/9/19 6:28 PM, Daniel P. Berrangé wrote: > > 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 ? > > The XICS interrupt controller models are used in two different machines, > sPAPR and PowerNV. > > > Feels like either they need to be merged, or they need to be split up > > and refactored even more to remove the mutual dependancy. > > yes or that some spapr_* definitions do not belong to the common xics.h > file. Yeah, These should move to a new xics_spapr.h or maybe even spapr.h for simplicity: void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt, uint32_t phandle); int xics_kvm_init(sPAPRMachineState *spapr, Error **errp); void xics_spapr_init(sPAPRMachineState *spapr); > > Thanks, > > C. > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 17:47 ` Greg Kurz @ 2019-01-10 6:47 ` Thomas Huth 0 siblings, 0 replies; 19+ messages in thread From: Thomas Huth @ 2019-01-10 6:47 UTC (permalink / raw) To: Greg Kurz, Cédric Le Goater Cc: Daniel P. Berrangé, peter.maydell, qemu-devel, Markus Armbruster, qemu-ppc, pbonzini, Richard Henderson On 2019-01-09 18:47, Greg Kurz wrote: > On Wed, 9 Jan 2019 18:40:48 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > >> On 1/9/19 6:28 PM, Daniel P. Berrangé wrote: >>> 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 ? >> >> The XICS interrupt controller models are used in two different machines, >> sPAPR and PowerNV. >> >>> Feels like either they need to be merged, or they need to be split up >>> and refactored even more to remove the mutual dependancy. >> >> yes or that some spapr_* definitions do not belong to the common xics.h >> file. > > Yeah, > > These should move to a new xics_spapr.h or maybe even spapr.h for simplicity: > > void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt, > uint32_t phandle); > int xics_kvm_init(sPAPRMachineState *spapr, Error **errp); > void xics_spapr_init(sPAPRMachineState *spapr); Ok, I'll move them to xics_spapr.h. Thomas ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 17:28 ` Daniel P. Berrangé 2019-01-09 17:40 ` Cédric Le Goater @ 2019-01-09 21:20 ` Paolo Bonzini 2019-01-09 21:25 ` Eric Blake 2019-01-10 6:46 ` Thomas Huth 1 sibling, 2 replies; 19+ messages in thread From: Paolo Bonzini @ 2019-01-09 21:20 UTC (permalink / raw) To: Daniel P. Berrangé, Cédric Le Goater Cc: Thomas Huth, qemu-devel, peter.maydell, Markus Armbruster, qemu-ppc, Richard Henderson On 09/01/19 18:28, Daniel P. Berrangé wrote: >> 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. If they include each other only for the typedefs, then prehaps the solution is to change the coding style and allow using struct in function prototypes. I'm pretty sure there are several examples of this already. Paolo ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 21:20 ` Paolo Bonzini @ 2019-01-09 21:25 ` Eric Blake 2019-01-09 21:31 ` Paolo Bonzini 2019-01-10 10:12 ` Daniel P. Berrangé 2019-01-10 6:46 ` Thomas Huth 1 sibling, 2 replies; 19+ messages in thread From: Eric Blake @ 2019-01-09 21:25 UTC (permalink / raw) To: Paolo Bonzini, Daniel P. Berrangé, Cédric Le Goater Cc: peter.maydell, Thomas Huth, qemu-devel, Markus Armbruster, qemu-ppc, Richard Henderson [-- Attachment #1: Type: text/plain, Size: 866 bytes --] On 1/9/19 3:20 PM, Paolo Bonzini wrote: > On 09/01/19 18:28, Daniel P. Berrangé wrote: >>> 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. > > If they include each other only for the typedefs, then prehaps the > solution is to change the coding style and allow using struct in > function prototypes. I'm pretty sure there are several examples of this > already. Or stick the typedef in <typedefs.h>, instead of trying to find (or create) some other common header. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 21:25 ` Eric Blake @ 2019-01-09 21:31 ` Paolo Bonzini 2019-01-10 10:12 ` Daniel P. Berrangé 1 sibling, 0 replies; 19+ messages in thread From: Paolo Bonzini @ 2019-01-09 21:31 UTC (permalink / raw) To: Eric Blake, Daniel P. Berrangé, Cédric Le Goater Cc: peter.maydell, Thomas Huth, qemu-devel, Markus Armbruster, qemu-ppc, Richard Henderson [-- Attachment #1: Type: text/plain, Size: 1128 bytes --] On 09/01/19 22:25, Eric Blake wrote: > On 1/9/19 3:20 PM, Paolo Bonzini wrote: >> On 09/01/19 18:28, Daniel P. Berrangé wrote: >>>> 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. >> >> If they include each other only for the typedefs, then prehaps the >> solution is to change the coding style and allow using struct in >> function prototypes. I'm pretty sure there are several examples of this >> already. > > Or stick the typedef in <typedefs.h>, instead of trying to find (or > create) some other common header. Putting typedefs for a very specific machine or device in typedefs.h doesn't seem too clean. Usually typedefs.h is used when things are shared by different subsystems (e.g. accelerator and device emulation, or frontend and backend), and I'm pretty sure that it could be made even smaller if someone was inclined to try. Paolo [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 21:25 ` Eric Blake 2019-01-09 21:31 ` Paolo Bonzini @ 2019-01-10 10:12 ` Daniel P. Berrangé 2019-01-10 12:12 ` Greg Kurz 1 sibling, 1 reply; 19+ messages in thread From: Daniel P. Berrangé @ 2019-01-10 10:12 UTC (permalink / raw) To: Eric Blake Cc: Paolo Bonzini, Cédric Le Goater, peter.maydell, Thomas Huth, qemu-devel, Markus Armbruster, qemu-ppc, Richard Henderson On Wed, Jan 09, 2019 at 03:25:53PM -0600, Eric Blake wrote: > On 1/9/19 3:20 PM, Paolo Bonzini wrote: > > On 09/01/19 18:28, Daniel P. Berrangé wrote: > >>> 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. > > > > If they include each other only for the typedefs, then prehaps the > > solution is to change the coding style and allow using struct in > > function prototypes. I'm pretty sure there are several examples of this > > already. > > Or stick the typedef in <typedefs.h>, instead of trying to find (or > create) some other common header. Probably better to just have a local spapr_types.h instead of polluting the global namespace. 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 :| ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-10 10:12 ` Daniel P. Berrangé @ 2019-01-10 12:12 ` Greg Kurz 0 siblings, 0 replies; 19+ messages in thread From: Greg Kurz @ 2019-01-10 12:12 UTC (permalink / raw) To: Daniel P. Berrangé Cc: Eric Blake, peter.maydell, Thomas Huth, qemu-devel, Markus Armbruster, qemu-ppc, Cédric Le Goater, Paolo Bonzini, Richard Henderson On Thu, 10 Jan 2019 10:12:43 +0000 Daniel P. Berrangé <berrange@redhat.com> wrote: > On Wed, Jan 09, 2019 at 03:25:53PM -0600, Eric Blake wrote: > > On 1/9/19 3:20 PM, Paolo Bonzini wrote: > > > On 09/01/19 18:28, Daniel P. Berrangé wrote: > > >>> 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. > > > > > > If they include each other only for the typedefs, then prehaps the > > > solution is to change the coding style and allow using struct in > > > function prototypes. I'm pretty sure there are several examples of this > > > already. > > > > Or stick the typedef in <typedefs.h>, instead of trying to find (or > > create) some other common header. > > Probably better to just have a local spapr_types.h instead of > polluting the global namespace. > I personally like this approach because it would allow to use the typedefs everywhere, for the sake of consistency. > > Regards, > Daniel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 21:20 ` Paolo Bonzini 2019-01-09 21:25 ` Eric Blake @ 2019-01-10 6:46 ` Thomas Huth 1 sibling, 0 replies; 19+ messages in thread From: Thomas Huth @ 2019-01-10 6:46 UTC (permalink / raw) To: Paolo Bonzini, Daniel P. Berrangé, Cédric Le Goater Cc: qemu-devel, peter.maydell, Markus Armbruster, qemu-ppc, Richard Henderson On 2019-01-09 22:20, Paolo Bonzini wrote: > On 09/01/19 18:28, Daniel P. Berrangé wrote: >>> 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. > > If they include each other only for the typedefs, then prehaps the > solution is to change the coding style and allow using struct in > function prototypes. I'm pretty sure there are several examples of this > already. I'm all in favor for this! Thomas ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 2019-01-09 16:39 ` [Qemu-devel] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 Thomas Huth 2019-01-09 17:13 ` [Qemu-devel] [Qemu-ppc] " Cédric Le Goater @ 2019-01-09 17:46 ` Cédric Le Goater 1 sibling, 0 replies; 19+ messages in thread From: Cédric Le Goater @ 2019-01-09 17:46 UTC (permalink / raw) To: Thomas Huth, qemu-devel Cc: peter.maydell, Daniel P. Berrangé, Markus Armbruster, qemu-ppc, pbonzini, Richard Henderson 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 */ > > void spapr_xive_hcall_init(sPAPRMachineState *spapr); > void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt, > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index 14afda1..a7f49a4 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -47,6 +47,8 @@ typedef struct ICSState ICSState; > typedef struct ICSIRQState ICSIRQState; > typedef struct XICSFabric XICSFabric; > > +#include "hw/ppc/spapr.h" > + > #define TYPE_ICP "icp" > #define ICP(obj) OBJECT_CHECK(ICPState, (obj), TYPE_ICP) > > @@ -200,8 +202,6 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon); > void ics_resend(ICSState *ics); > void icp_resend(ICPState *ss); I think the definitions below belong to another header file, may be "xics_spapr.h". That would be cleaner. Thanks, C. > -typedef struct sPAPRMachineState sPAPRMachineState; > - > void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt, > uint32_t phandle); > int xics_kvm_init(sPAPRMachineState *spapr, Error **errp); > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] configure: Force the C standard to gnu99 2019-01-09 16:39 [Qemu-devel] [PATCH v3 0/2] Force the C standard to gnu99 Thomas Huth 2019-01-09 16:39 ` [Qemu-devel] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 Thomas Huth @ 2019-01-09 16:39 ` Thomas Huth 2019-01-09 18:10 ` Philippe Mathieu-Daudé 2019-01-09 21:23 ` Richard Henderson 1 sibling, 2 replies; 19+ messages in thread From: Thomas Huth @ 2019-01-09 16:39 UTC (permalink / raw) To: qemu-devel Cc: Richard Henderson, Daniel P. Berrangé, pbonzini, peter.maydell, Markus Armbruster, qemu-ppc Different versions of GCC and Clang use different versions of the C standard. This repeatedly caused problems already, e.g. with duplicated typedefs: https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05829.html or with for-loop variable initializers: https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00237.html To avoid these problems, we should enforce the C language version to the same level for all compilers. Since our minimum compiler versions is GCC v4.8, our best option is "gnu99" for C code right now ("gnu17" is not available there yet, and "gnu11" is marked as "experimental"), and "gnu++98" for the few C++ code that we have in the repository. Signed-off-by: Thomas Huth <thuth@redhat.com> --- v3: Compile C++ code with -std=gnu++98 configure | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/configure b/configure index b9f34af..448dbc8 100755 --- a/configure +++ b/configure @@ -107,6 +107,9 @@ update_cxxflags() { -Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\ -Wold-style-declaration|-Wold-style-definition|-Wredundant-decls) ;; + -std=gnu99) + QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }"-std=gnu++98" + ;; *) QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }$arg ;; @@ -585,7 +588,7 @@ ARFLAGS="${ARFLAGS-rv}" # left shift of signed integers is well defined and has the expected # 2s-complement style results. (Both clang and gcc agree that it # provides these semantics.) -QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS" +QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv -std=gnu99 $QEMU_CFLAGS" QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS" QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS" QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS" -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] configure: Force the C standard to gnu99 2019-01-09 16:39 ` [Qemu-devel] [PATCH v3 2/2] configure: Force the C standard to gnu99 Thomas Huth @ 2019-01-09 18:10 ` Philippe Mathieu-Daudé 2019-01-09 21:26 ` Paolo Bonzini 2019-01-09 21:23 ` Richard Henderson 1 sibling, 1 reply; 19+ messages in thread From: Philippe Mathieu-Daudé @ 2019-01-09 18:10 UTC (permalink / raw) To: Thomas Huth, qemu-devel Cc: peter.maydell, Markus Armbruster, qemu-ppc, pbonzini, Richard Henderson On 1/9/19 5:39 PM, Thomas Huth wrote: > Different versions of GCC and Clang use different versions of the C standard. > This repeatedly caused problems already, e.g. with duplicated typedefs: > > https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05829.html > > or with for-loop variable initializers: > > https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00237.html > > To avoid these problems, we should enforce the C language version to the > same level for all compilers. Since our minimum compiler versions is > GCC v4.8, our best option is "gnu99" for C code right now ("gnu17" is not > available there yet, and "gnu11" is marked as "experimental"), and "gnu++98" > for the few C++ code that we have in the repository. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Raising from a 30 years old stantard to a 20 years old one is a great improvement (I used to be a defender of the C89, but I evolved and now admit C99 has helpful features). Using '-std=gnu++98' for g++ v4.8 looks like a good compromise to the issues Daniel mentioned (still 'experimental'). Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > v3: Compile C++ code with -std=gnu++98 > > configure | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/configure b/configure > index b9f34af..448dbc8 100755 > --- a/configure > +++ b/configure > @@ -107,6 +107,9 @@ update_cxxflags() { > -Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\ > -Wold-style-declaration|-Wold-style-definition|-Wredundant-decls) > ;; > + -std=gnu99) > + QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }"-std=gnu++98" > + ;; > *) > QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }$arg > ;; > @@ -585,7 +588,7 @@ ARFLAGS="${ARFLAGS-rv}" > # left shift of signed integers is well defined and has the expected > # 2s-complement style results. (Both clang and gcc agree that it > # provides these semantics.) > -QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS" > +QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv -std=gnu99 $QEMU_CFLAGS" > QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS" > QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS" > QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS" > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] configure: Force the C standard to gnu99 2019-01-09 18:10 ` Philippe Mathieu-Daudé @ 2019-01-09 21:26 ` Paolo Bonzini 2019-01-10 6:50 ` Thomas Huth 0 siblings, 1 reply; 19+ messages in thread From: Paolo Bonzini @ 2019-01-09 21:26 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Thomas Huth, qemu-devel Cc: peter.maydell, Markus Armbruster, qemu-ppc, Richard Henderson On 09/01/19 19:10, Philippe Mathieu-Daudé wrote: > Using '-std=gnu++98' for g++ v4.8 looks like a good compromise to the > issues Daniel mentioned (still 'experimental'). > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> C++11 has many new features that almost make it an entirely different language (the main being rvalue references, and type inference with auto and decltype). If it works for GCC 4.8, I would prefer using that instead of g++98. Paolo ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] configure: Force the C standard to gnu99 2019-01-09 21:26 ` Paolo Bonzini @ 2019-01-10 6:50 ` Thomas Huth 0 siblings, 0 replies; 19+ messages in thread From: Thomas Huth @ 2019-01-10 6:50 UTC (permalink / raw) To: Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel Cc: peter.maydell, Markus Armbruster, qemu-ppc, Richard Henderson On 2019-01-09 22:26, Paolo Bonzini wrote: > On 09/01/19 19:10, Philippe Mathieu-Daudé wrote: >> Using '-std=gnu++98' for g++ v4.8 looks like a good compromise to the >> issues Daniel mentioned (still 'experimental'). >> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > C++11 has many new features that almost make it an entirely different > language (the main being rvalue references, and type inference with auto > and decltype). If it works for GCC 4.8, I would prefer using that > instead of g++98. I think we can revisit that setting when we really want to introduce code that needs it. Currently it does not seem to be necessary, so that does not justify to use an "experimental" feature of GCC 4.8, I guess. Thomas ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] configure: Force the C standard to gnu99 2019-01-09 16:39 ` [Qemu-devel] [PATCH v3 2/2] configure: Force the C standard to gnu99 Thomas Huth 2019-01-09 18:10 ` Philippe Mathieu-Daudé @ 2019-01-09 21:23 ` Richard Henderson 1 sibling, 0 replies; 19+ messages in thread From: Richard Henderson @ 2019-01-09 21:23 UTC (permalink / raw) To: Thomas Huth, qemu-devel Cc: peter.maydell, Markus Armbruster, qemu-ppc, pbonzini, Richard Henderson On 1/10/19 3:39 AM, Thomas Huth wrote: > Different versions of GCC and Clang use different versions of the C standard. > This repeatedly caused problems already, e.g. with duplicated typedefs: > > https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05829.html > > or with for-loop variable initializers: > > https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00237.html > > To avoid these problems, we should enforce the C language version to the > same level for all compilers. Since our minimum compiler versions is > GCC v4.8, our best option is "gnu99" for C code right now ("gnu17" is not > available there yet, and "gnu11" is marked as "experimental"), and "gnu++98" > for the few C++ code that we have in the repository. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v3: Compile C++ code with -std=gnu++98 Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-01-10 12:13 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-01-09 16:39 [Qemu-devel] [PATCH v3 0/2] Force the C standard to gnu99 Thomas Huth 2019-01-09 16:39 ` [Qemu-devel] [PATCH 1/2] include/hw/ppc: Fix compilation with clang -std=gnu99 Thomas Huth 2019-01-09 17:13 ` [Qemu-devel] [Qemu-ppc] " Cédric Le Goater 2019-01-09 17:28 ` Daniel P. Berrangé 2019-01-09 17:40 ` Cédric Le Goater 2019-01-09 17:47 ` Greg Kurz 2019-01-10 6:47 ` Thomas Huth 2019-01-09 21:20 ` Paolo Bonzini 2019-01-09 21:25 ` Eric Blake 2019-01-09 21:31 ` Paolo Bonzini 2019-01-10 10:12 ` Daniel P. Berrangé 2019-01-10 12:12 ` Greg Kurz 2019-01-10 6:46 ` Thomas Huth 2019-01-09 17:46 ` Cédric Le Goater 2019-01-09 16:39 ` [Qemu-devel] [PATCH v3 2/2] configure: Force the C standard to gnu99 Thomas Huth 2019-01-09 18:10 ` Philippe Mathieu-Daudé 2019-01-09 21:26 ` Paolo Bonzini 2019-01-10 6:50 ` Thomas Huth 2019-01-09 21:23 ` Richard Henderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).