Re: [Xen-devel] [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 12 Feb 2016, Jan Beulich wrote:
> >>> On 12.02.16 at 12:37, <stefano.stabellini@xxxxxxxxxxxxx> 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 <asm/processor.h>.
> 
> 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.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel