Re: [Xen-devel] [PATCH RFC 01/25] Include some header files that are not automatically included on all archs

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 7 Dec 2011, Jan Beulich wrote:
> >>> On 06.12.11 at 19:19, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> > --- a/xen/common/grant_table.c
> > +++ b/xen/common/grant_table.c
> > @@ -24,6 +24,7 @@
> >   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  
> > USA
> >   */
> >  
> > +#include <asm/flushtlb.h>
> 
> It's pretty uncommon to have asm/ headers included before xen/ ones,
> and it's definitely wrong to do so before xen/config.h.
> 
> >  #include <xen/config.h>
> >  #include <xen/iocap.h>
> >  #include <xen/lib.h>
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -23,9 +23,7 @@
> >  #include <xen/tmem_xen.h>
> >  #include <asm/current.h>
> >  #include <asm/hardirq.h>
> > -#ifdef CONFIG_X86
> > -# include <asm/p2m.h>
> > -#endif
> > +#include <asm/p2m.h>
> 
> This header doesn't exist on ia64; if you had looked in the history
> why the #ifdef got introduced, you would have noticed. So either
> we need to introduce something like CONFIG_P2M, or ia64 needs
> to get a stub header, or the conditional above needs to simply be
> extended.
> 
> >  #include <xen/numa.h>
> >  #include <public/memory.h>
> >  #include <xsm/xsm.h>
> > --- a/xen/include/xen/tmem_xen.h
> > +++ b/xen/include/xen/tmem_xen.h
> > @@ -11,6 +11,7 @@
> >  
> >  #include <xen/config.h>
> >  #include <xen/mm.h> /* heap alloc/free */
> > +#include <xen/pfn.h> /* heap alloc/free */
> 
> The comment is certainly wrong - correct it or remove it.
> 

Thanks for the review, I agree with all your comments.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel