Re: [PATCH][12/37] Clean up duplicate includes in drivers/net/

["Jesper Juhl" <jesper.juhl@gmail.com>]

On 21/07/07, Chris Snook <csnook@redhat.com> wrote:
> Jesper Juhl wrote:
> > Hi,
> >
> > This patch cleans up duplicate includes in
> >        drivers/net/
> >
> >
> > Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
> > ---
> >
> > diff --git a/drivers/net/atl1/atl1_main.c b/drivers/net/atl1/atl1_main.c
> > index fd1e156..4a18b88 100644
> > --- a/drivers/net/atl1/atl1_main.c
> > +++ b/drivers/net/atl1/atl1_main.c
> > @@ -75,7 +75,6 @@
> >  #include <linux/compiler.h>
> >  #include <linux/delay.h>
> >  #include <linux/mii.h>
> > -#include <linux/interrupt.h>
> >  #include <net/checksum.h>
> >
> >  #include <asm/atomic.h>
>
> Define "duplicate".  I ask because this patch just got posted a few days ago:
>

duplicate == present more than once in the same source file.
Did you try looking at the includes in the source file the patch modifies?


> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>   drivers/net/atl1/atl1_main.c |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/atl1/atl1_main.c b/drivers/net/atl1/atl1_main.c
> index 4a18b88..fd1e156 100644
> --- a/drivers/net/atl1/atl1_main.c
> +++ b/drivers/net/atl1/atl1_main.c
> @@ -75,6 +75,7 @@
>   #include <linux/compiler.h>
>   #include <linux/delay.h>
>   #include <linux/mii.h>
> +#include <linux/interrupt.h>
>   #include <net/checksum.h>
>
>   #include <asm/atomic.h>
>
> I've always been under the impression that one should include all the files
> whose contents you use directly, because other includes that happen to include
> them might no longer need to in the future and cease including them.

I agree completely. But that's completely beside the point here.

Before Al's patch, drivers/net/atl1/atl1_main.c already contained
"#include <linux/interrupt.h>".

> You can
> fight it out with Al if you feel like it.  I'm keeping the rest of the CC list
> because the other maintainers might have similar feelings about the
> appropriateness of these includes in their drivers.
>

Take a look at the file. These are the includes at the top of
drivers/net/atl1/atl1_main.c :

...
#include <linux/types.h>
#include <linux/netdevice.h>
#include <linux/pci.h>
#include <linux/spinlock.h>
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/skbuff.h>
#include <linux/etherdevice.h>
#include <linux/if_vlan.h>
#include <linux/irqreturn.h>
#include <linux/workqueue.h>
#include <linux/timer.h>
#include <linux/jiffies.h>
#include <linux/hardirq.h>
#include <linux/interrupt.h>          <--- Here we have linux/interrupt.h
#include <linux/irqflags.h>
#include <linux/dma-mapping.h>
#include <linux/net.h>
#include <linux/pm.h>
#include <linux/in.h>
#include <linux/ip.h>
#include <linux/tcp.h>
#include <linux/compiler.h>
#include <linux/delay.h>
#include <linux/mii.h>
#include <linux/interrupt.h>          <--- And here we include it again.
#include <net/checksum.h>

#include <asm/atomic.h>
#include <asm/byteorder.h>

#include "atl1.h"
...

Now please tell me why it makes sense to include the same header twice
and why my patch that removes the duplicate does not make sense.

-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html