Re: [Qemu-ppc] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode

[Greg Kurz]

On Thu, 17 Jan 2019 08:01:25 +0100
Thomas Huth <address@hidden> wrote:

> On 2019-01-16 14:29, Cédric Le Goater wrote:
> > On 1/16/19 12:47 PM, Thomas Huth wrote:  
> >> On 2019-01-16 12:43, Cédric Le Goater wrote:  
> >>> On 1/11/19 9:17 AM, Thomas Huth wrote:  
> >>>> When compiling the ppc code with clang and -std=gnu99, there are a
> >>>> couple of warnings/errors like this one:
> >>>>
> >>>>   CC      ppc64-softmmu/hw/intc/xics.o
> >>>> In file included from hw/intc/xics.c:35:
> >>>> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' 
> >>>> is a C11 feature
> >>>>       [-Werror,-Wtypedef-redefinition]
> >>>> typedef struct ICPState ICPState;
> >>>>                         ^
> >>>> target/ppc/cpu.h:1181:25: note: previous definition is here
> >>>> typedef struct ICPState ICPState;
> >>>>                         ^
> >>>> Work around the problems by including the proper headers instead.  
> >>>
> >>> Thomas,
> >>>
> >>>
> >>> After a closer look, I think we should use 'void *' under PowerPCCPU 
> >>> as it was the case before I introduced the second interrupt presenter.  
> >>
> >> If you don't like the #includes, why not simply do anonymous struct
> >> forward declarations here? I think that would be better than "void *".  
> > 
> > yes.
> >    
> >>> That's a bigger change reverting bits of already merged patches. I can
> >>> take care of it if you prefer.   
> >>
> >> Could I keep the current patch in my series so that I can get the
> >> patches finally merged? You could then do any clean up that you like on
> >> top of it, ok?  
> > 
> > OK. 
> > 
> > See below the patch I would propose. Compiled tested with clang -std=gnu99. 
> >  
> [...]
> > @@ -1204,8 +1199,8 @@ struct PowerPCCPU {
> >      int32_t node_id; /* NUMA node this CPU belongs to */
> >      PPCHash64Options *hash64_opts;
> >  #ifndef CONFIG_USER_ONLY
> > -    ICPState *icp;
> > -    XiveTCTX *tctx;
> > +    struct ICPState *icp;
> > +    struct XiveTCTX *tctx;
> >  #endif  
> 
> That's pretty much what I had in an earlier version of my patch:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg01810.html
> 
> But Greg did not like it:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg01893.html
> 

Yeah I didn't but the #includes bring even more troubles so I won't object
if we go for struct :) For the long term, I still think that icp and tctx
should be hidden behind void *machine_data in a per-machine struct.

>  Thomas