From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47258) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dnQZv-0000sS-Oy for qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:36:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dnQZq-0003v7-Pe for qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:36:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19459) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dnQZq-0003uw-Ji for qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:36:14 -0400 References: <20170830170601.15855-1-david@redhat.com> <20170830170601.15855-4-david@redhat.com> <09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com> <55b99307-b817-1cb2-f5c8-901b877735cc@redhat.com> <63bc0433-dd9f-2b75-7885-5460949c821b@redhat.com> From: David Hildenbrand Message-ID: <8a4d349c-33c5-0e63-d11b-274d64e854fa@redhat.com> Date: Thu, 31 Aug 2017 16:36:09 +0200 MIME-Version: 1.0 In-Reply-To: <63bc0433-dd9f-2b75-7885-5460949c821b@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth , qemu-devel@nongnu.org Cc: Richard Henderson , Aurelien Jarno , cohuck@redhat.com, borntraeger@de.ibm.com, Alexander Graf 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