Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state

[Thomas Huth]

On 31.08.2017 16:23, David Hildenbrand wrote:
> 
>>> +struct S390CPU;
>>
>> You define a "struct S390CPU" here ...
>>
>>>  typedef struct S390CcwMachineState {
>>>      /*< private >*/
>>>      MachineState parent_obj;
>>>  
>>>      /*< public >*/
>>> +    S390CPU **cpus;
>>
>> ... but use the typedef'ed S390CPU here ... looks somewhat suspicious, I
>> wonder whether the typedef is really in the right place?
> 
> General question: how much do we care about headers that are not consistent?
> 
> E.g. shall I forward declare or simply ignore if compilers don't bite me?

My remark was not so much about your patch, but about the original
definition instead: "struct S390CPU" is declared in target/s390x/cpu.h,
but "typedef struct S390CPU S390CPU" is in target/s390x/cpu-qom.h. I
think they should rather be declared in the same header file instead. Or
your "struct S390CPU;" forward declaration should go into cpu-qom.h
instead, right in front of the typedef.

 Thomas