From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs; Date: Fri, 12 Feb 2016 12:46:47 +0000 Message-ID: References: <1455246507-5589-1-git-send-email-konrad.wilk@oracle.com> <1455246507-5589-2-git-send-email-konrad.wilk@oracle.com> <56BDDC2E02000078000D1647@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aUD7y-0005Ko-DO for xen-devel@lists.xenproject.org; Fri, 12 Feb 2016 12:47:14 +0000 In-Reply-To: <56BDDC2E02000078000D1647@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: wei.liu2@citrix.com, ian.campbell@citrix.com, Stefano Stabellini , ian.jackson@eu.citrix.com, stefano.stabellini@citrix.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Fri, 12 Feb 2016, Jan Beulich wrote: > >>> On 12.02.16 at 12:37, wrote: > > On Thu, 11 Feb 2016, Konrad Rzeszutek Wilk wrote: > >> --- a/xen/include/xen/keyhandler.h > >> +++ b/xen/include/xen/keyhandler.h > >> @@ -19,6 +19,7 @@ > >> */ > >> typedef void (keyhandler_fn_t)(unsigned char key); > >> > >> +struct cpu_user_regs; > >> /* > >> * Callback type for irq_keyhandler. > >> * > > > > I think that the right fix would be to #include . > > I disagree - for one this isn't where the structure gets defined, Ah, sorry, it is on ARM (actually to be precise, the header files are asm-arm/arm64/processor.h and asm-arm/arm32/processor.h, included by asm-arm/processor.h depending on the architecture). I see now that the corresponding x86 header is actually arch-x86/xen.h. > and then this is the "include everything everywhere" attitude > which tends to needlessly slow down builds (avoiding the need > to include everything everywhere is actually one of the > purposes of such forward declarations, which allows going as > far as having the full definition in just a single source file). Fair enough, but keyhandler.h directly uses struct cpu_user_regs as parameter in two functions, so I think it would be right to include an header file with the definition. It's too bad that the names for the ARM headers and the x86 headers don't match. Given that, I understand why Konrad went with this solution instead, and I am OK with it.