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

[David Hildenbrand <david@redhat.com>]

On 31.08.2017 16:31, Thomas Huth wrote:
> 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

I agree, will have a look.

> your "struct S390CPU;" forward declaration should go into cpu-qom.h
> instead, right in front of the typedef.
> 

Let me rephrase my question:

include/hw/s390x/s390-virtio-ccw.h does not include cpu.h/cpu-qom.h

If compilers don't complain, do we have to forward declare at all? (I
think it is cleaner, but I would like to know what is suggested)

>  Thomas
> 


-- 

Thanks,

David