[PATCH] net: move ethtool include to where its used

[PATCH] net: move ethtool include to where its used

[Mike Frysinger]

The linux/mii.h headers has long been standalone for userspace.  But a
recent commit added linux/ethtool.h to the include list even when it
isn't used.  Since we only need this with __KERNEL__, move the include
down to the right spot.  Now userland no longer needs to pull this in.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 include/linux/mii.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/mii.h b/include/linux/mii.h
index 2783eca..415b397 100644
--- a/include/linux/mii.h
+++ b/include/linux/mii.h
@@ -9,7 +9,6 @@
 #define __LINUX_MII_H__
 
 #include <linux/types.h>
-#include <linux/ethtool.h>
 
 /* Generic MII registers. */
 #define MII_BMCR		0x00	/* Basic mode control register */
@@ -151,6 +150,7 @@ struct mii_ioctl_data {
 
 #ifdef __KERNEL__
 
+#include <linux/ethtool.h>
 #include <linux/if.h>
 
 struct ethtool_cmd;
-- 
1.7.8.4

Re: [PATCH] net: move ethtool include to where its used

[David Miller]

From: Mike Frysinger <vapier@gentoo.org>
Date: Fri,  3 Feb 2012 06:17:02 -0500

> The linux/mii.h headers has long been standalone for userspace.  But a
> recent commit added linux/ethtool.h to the include list even when it
> isn't used.  Since we only need this with __KERNEL__, move the include
> down to the right spot.  Now userland no longer needs to pull this in.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

I don't understand what the problem is.

linux/ethtool.h is a safely exported header file for userspace,
so if linux/mii.h now starts to include it this should make no
difference whatsoever for userspace unless userspace does something
bogus which in turn would mean it's almost certainly userspace
which is at fault here no the kernel headers.

You don't describe the exact nature of the failure nor have you shown
a specific actual failure, so we can only guess what the real problem
is.  And that's another thing that makes this patch submission bogus.

This change seems arbitrary and not necessary, therefore I am not
applying it.

Re: [PATCH] net: move ethtool include to where its used

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1030 bytes --]

On Saturday 04 February 2012 16:23:31 David Miller wrote:
> From: Mike Frysinger <vapier@gentoo.org>
> > The linux/mii.h headers has long been standalone for userspace.  But a
> > recent commit added linux/ethtool.h to the include list even when it
> > isn't used.  Since we only need this with __KERNEL__, move the include
> > down to the right spot.  Now userland no longer needs to pull this in.
> 
> I don't understand what the problem is.

the exported linux/mii.h userspace code doesn't need linux/ethtool.h, thus it 
should not need to include it

> linux/ethtool.h is a safely exported header file for userspace,
> so if linux/mii.h now starts to include it this should make no
> difference whatsoever for userspace unless userspace does something
> bogus which in turn would mean it's almost certainly userspace
> which is at fault here no the kernel headers.

by the same token, you could have linux/mii.h pointlessly include a lot of 
other unused headers.  they're still unused and a waste.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] net: move ethtool include to where its used

[David Miller]

From: Mike Frysinger <vapier@gentoo.org>
Date: Sat, 4 Feb 2012 21:20:07 -0500

> On Saturday 04 February 2012 16:23:31 David Miller wrote:
>> From: Mike Frysinger <vapier@gentoo.org>
>> > The linux/mii.h headers has long been standalone for userspace.  But a
>> > recent commit added linux/ethtool.h to the include list even when it
>> > isn't used.  Since we only need this with __KERNEL__, move the include
>> > down to the right spot.  Now userland no longer needs to pull this in.
>> 
>> I don't understand what the problem is.
> 
> the exported linux/mii.h userspace code doesn't need linux/ethtool.h, thus it 
> should not need to include it

So there's not bug, you're just twiddling.

I don't think there should be any rule that just because the stuff
needed by a header is in __KERNEL__ we have to put the include in
there too, if the header in question is userspace clean itself.

It's so damn ugly, and error prone during audits (ie. easy to miss),
to have include directives not at the top of the file.

So I'm really not interested at all in applying patches like this
one, sorry.