Re: [PATCH 2/2] arch/x86/xen: Silence compiler warnings

From: Konrad Rzeszutek Wilk
Date: Fri Jul 11 2014 - 21:33:47 EST


. snip ..
> > > Please loot at arch/x86/xen/enlighten.c:xen_check_mwait() and
> > > arch/x86/xen/enlighten.c:xen_boot_params_init_edd() (probably
> > > there are more stuff like that around). As I can see this is fairly
> > > common solution and probably compiler cope with it quite well.
> > >
> >
> > Those are some examples of some rather bad examples.
>
> What is wrong with them?

#ifdef should not be in C files. It is making the code a bit of a mess.
>
> > The way that is preferred in the Linux code is to have the ifdef in headers.
> >
> > See
> > http://lxr.free-electrons.com/source/arch/x86/include/asm/xen/swiotlb-xen.h
> > Or
> > http://lxr.free-electrons.com/source/arch/x86/include/asm/xen/pci.h
> >
> > You can create a similar file there and for the 32 bit implementation just make an empty static function.
> >
> > The 64 bit implementation has to be somewhere. Can it be in the Xen EFI file which is only compiled on 64 bit platforms?
>
> OK, this (putting declaration/definition in *.h file) makes sens if you
> declare/define functions which must be called from different places.

Right.
> However, xen_efi_init() is called only once in arch/x86/xen/enlighten.c.

And the vga (see arch/x86/xen/vga.c) is also called only once.

> Of course, I could define this function here in similar way like it is done
> in above headers but it take a bit more place. However, if you wish why not.

I was thinking something along this (not compile tested):