diff mbox

net: move ethtool include to where its used

Message ID 1328267822-25690-1-git-send-email-vapier@gentoo.org
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Mike Frysinger Feb. 3, 2012, 11:17 a.m. UTC
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(-)

Comments

David Miller Feb. 4, 2012, 9:23 p.m. UTC | #1
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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Frysinger Feb. 5, 2012, 2:20 a.m. UTC | #2
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
David Miller Feb. 5, 2012, 5:24 a.m. UTC | #3
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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;