[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state |
Date: |
Thu, 31 Aug 2017 16:45:44 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 31.08.2017 16:36, David Hildenbrand wrote:
> 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)
Well, doing a forward declaration with "struct S390CPU;" and then using
the typedef'd "S390CPU" without "struct" does not make much sense -
these are two "different" types. If you can use "S390CPU" here without
the "struct S390CPU;" declaration, that means the cpu-qom.h header got
included indirectly already - so I'd suggest to simply remove the
"struct S390CPU;" forward declaration from your patch.
Thomas
- [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, (continued)
- [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, David Hildenbrand, 2017/08/30
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, Thomas Huth, 2017/08/30
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, David Hildenbrand, 2017/08/31
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, Cornelia Huck, 2017/08/31
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, David Hildenbrand, 2017/08/31
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, Cornelia Huck, 2017/08/31
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, David Hildenbrand, 2017/08/31
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, David Hildenbrand, 2017/08/31
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, Thomas Huth, 2017/08/31
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state, David Hildenbrand, 2017/08/31
- Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state,
Thomas Huth <=
[Qemu-devel] [PATCH v1 04/11] s390x: get rid of s390-virtio.c, David Hildenbrand, 2017/08/30
[Qemu-devel] [PATCH v1 06/11] target/s390x: cleanup cpu number/address handling, David Hildenbrand, 2017/08/30