Xen devel - Mar 2006 - Re: [Xen-changelog] Fix the implicit declaration of function `swiotlb_init'' warning, by including

On Tue, Mar 21, 2006 at 01:14:15PM +0000, Xen patchbot -unstable wrote:
> diff -r a013fa6ee37c -r 14c26df4f33c
linux-2.6-xen-sparse/arch/i386/mm/init-xen.c
> --- a/linux-2.6-xen-sparse/arch/i386/mm/init-xen.c	Tue Mar 21 10:56:11 2006
> +++ b/linux-2.6-xen-sparse/arch/i386/mm/init-xen.c	Tue Mar 21 11:21:08 2006
> @@ -42,6 +42,11 @@
>  #include <asm/tlbflush.h>
>  #include <asm/sections.h>
>  #include <asm/hypervisor.h>
> +#if defined(CONFIG_SWIOTLB)
> +#include <linux/dma-mapping.h>
> +#include <asm/scatterlist.h>
> +#include <asm/swiotlb.h>
> +#endif
Hmm, the right way(TM) to do it is as follows (compile tested
only). We''re firmly in nit-picking territory, but it''s one
less thing
that will be pointed out when we finally submit it for upstream
inclusion.

Signed-off-by: Muli Ben-Yehuda <mulix@mulix.org>

diff -r 045bee6e1ebd linux-2.6-xen-sparse/arch/i386/mm/init-xen.c
--- a/linux-2.6-xen-sparse/arch/i386/mm/init-xen.c	Tue Mar 21 12:26:38 2006
+0100
+++ b/linux-2.6-xen-sparse/arch/i386/mm/init-xen.c	Tue Mar 21 20:43:54 2006
+0200
@@ -29,6 +29,8 @@
 #include <linux/efi.h>
 #include <linux/memory_hotplug.h>
 #include <linux/initrd.h>
+#include <linux/dma-mapping.h>
+#include <linux/scatterlist.h>
 
 #include <asm/processor.h>
 #include <asm/system.h>
@@ -42,11 +44,7 @@
 #include <asm/tlbflush.h>
 #include <asm/sections.h>
 #include <asm/hypervisor.h>
-#if defined(CONFIG_SWIOTLB)
-#include <linux/dma-mapping.h>
-#include <asm/scatterlist.h>
 #include <asm/swiotlb.h>
-#endif
 
 extern unsigned long *contiguous_bitmap;
 

-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/


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

On Tue, Mar 21, 2006 at 08:46:01PM +0200, Muli Ben-Yehuda wrote:
> On Tue, Mar 21, 2006 at 01:14:15PM +0000, Xen patchbot -unstable wrote:
> 
> > diff -r a013fa6ee37c -r 14c26df4f33c
linux-2.6-xen-sparse/arch/i386/mm/init-xen.c
> > --- a/linux-2.6-xen-sparse/arch/i386/mm/init-xen.c	Tue Mar 21 10:56:11
2006
> > +++ b/linux-2.6-xen-sparse/arch/i386/mm/init-xen.c	Tue Mar 21 11:21:08
2006
> > @@ -42,6 +42,11 @@
> >  #include <asm/tlbflush.h>
> >  #include <asm/sections.h>
> >  #include <asm/hypervisor.h>
> > +#if defined(CONFIG_SWIOTLB)
> > +#include <linux/dma-mapping.h>
> > +#include <asm/scatterlist.h>
> > +#include <asm/swiotlb.h>
> > +#endif
> 
> Hmm, the right way(TM) to do it is as follows (compile tested
> only). We''re firmly in nit-picking territory, but it''s
one less thing
> that will be pointed out when we finally submit it for upstream
> inclusion.
> 
> Signed-off-by: Muli Ben-Yehuda <mulix@mulix.org>
> 
> diff -r 045bee6e1ebd linux-2.6-xen-sparse/arch/i386/mm/init-xen.c
> --- a/linux-2.6-xen-sparse/arch/i386/mm/init-xen.c	Tue Mar 21 12:26:38 2006
+0100
> +++ b/linux-2.6-xen-sparse/arch/i386/mm/init-xen.c	Tue Mar 21 20:43:54 2006
+0200
> @@ -29,6 +29,8 @@
>  #include <linux/efi.h>
>  #include <linux/memory_hotplug.h>
>  #include <linux/initrd.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/scatterlist.h>
>  
>  #include <asm/processor.h>
>  #include <asm/system.h>
> @@ -42,11 +44,7 @@
>  #include <asm/tlbflush.h>
>  #include <asm/sections.h>
>  #include <asm/hypervisor.h>
> -#if defined(CONFIG_SWIOTLB)
> -#include <linux/dma-mapping.h>
> -#include <asm/scatterlist.h>
>  #include <asm/swiotlb.h>
> -#endif
>  
>  extern unsigned long *contiguous_bitmap;
Why, dare I ask?

Ewan.

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

On Tue, Mar 21, 2006 at 07:53:57PM +0000, Ewan Mellor
wrote:
> 
> Why, dare I ask?
There are two conventions here: no #ifdefs in C files if possible, and
#include <linux/xxx> comes before #include <asm/xxx>. The first one
helps readability and maintainability, the second one helps
readability and stems from letting arch specific include files
overrride generic include files.

But really, it''s just the accepted kernel idiom, and if we want
upstream inclusion, it pays to play by the rules.

Cheers,
Muli
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/


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