linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ethtool.h should use userspace-accessible types
@ 2004-03-12  1:54 Eric Brower
  2004-03-12  2:35 ` Tim Hockin
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Eric Brower @ 2004-03-12  1:54 UTC (permalink / raw)
  To: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

Attached is a patch to 2.4's ethtool.h to use appropriate, 
userspace-accessible data types (__u8 and friends, rather than u8 and 
friends).

I've also posted a patch against ethtool-1.8 to the sf.net gkernel site 
that cleans-up ethtool-copy.h and the rest in the same manner.

My motivation for the patch is to allow a userspace program to cleanly 
call ethtool ioctls without the typedef'ing used within the ethtool 
package.

Please cc me with responses, as I am not subscribed to lkml.

Thanks,
E

[-- Attachment #2: 2.4_ethtool_types.patch --]
[-- Type: text/plain, Size: 12529 bytes --]

===== include/linux/ethtool.h 1.18 vs edited =====
--- 1.18/include/linux/ethtool.h	Sun Oct 12 04:18:20 2003
+++ edited/include/linux/ethtool.h	Thu Mar 11 14:02:38 2004
@@ -15,24 +15,24 @@
 
 /* This should work for both 32 and 64 bit userland. */
 struct ethtool_cmd {
-	u32	cmd;
-	u32	supported;	/* Features this interface supports */
-	u32	advertising;	/* Features this interface advertises */
-	u16	speed;		/* The forced speed, 10Mb, 100Mb, gigabit */
-	u8	duplex;		/* Duplex, half or full */
-	u8	port;		/* Which connector port */
-	u8	phy_address;
-	u8	transceiver;	/* Which tranceiver to use */
-	u8	autoneg;	/* Enable or disable autonegotiation */
-	u32	maxtxpkt;	/* Tx pkts before generating tx int */
-	u32	maxrxpkt;	/* Rx pkts before generating rx int */
-	u32	reserved[4];
+	__u32	cmd;
+	__u32	supported;	/* Features this interface supports */
+	__u32	advertising;	/* Features this interface advertises */
+	__u16	speed;		/* The forced speed, 10Mb, 100Mb, gigabit */
+	__u8	duplex;		/* Duplex, half or full */
+	__u8	port;		/* Which connector port */
+	__u8	phy_address;
+	__u8	transceiver;	/* Which tranceiver to use */
+	__u8	autoneg;	/* Enable or disable autonegotiation */
+	__u32	maxtxpkt;	/* Tx pkts before generating tx int */
+	__u32	maxrxpkt;	/* Rx pkts before generating rx int */
+	__u32	reserved[4];
 };
 
 #define ETHTOOL_BUSINFO_LEN	32
 /* these strings are set to whatever the driver author decides... */
 struct ethtool_drvinfo {
-	u32	cmd;
+	__u32	cmd;
 	char	driver[32];	/* driver short name, "tulip", "eepro100" */
 	char	version[32];	/* driver version string */
 	char	fw_version[32];	/* firmware version string, if applicable */
@@ -40,53 +40,53 @@
 				/* For PCI devices, use pci_dev->slot_name. */
 	char	reserved1[32];
 	char	reserved2[16];
-	u32	n_stats;	/* number of u64's from ETHTOOL_GSTATS */
-	u32	testinfo_len;
-	u32	eedump_len;	/* Size of data from ETHTOOL_GEEPROM (bytes) */
-	u32	regdump_len;	/* Size of data from ETHTOOL_GREGS (bytes) */
+	__u32	n_stats;	/* number of __u64's from ETHTOOL_GSTATS */
+	__u32	testinfo_len;
+	__u32	eedump_len;	/* Size of data from ETHTOOL_GEEPROM (bytes) */
+	__u32	regdump_len;	/* Size of data from ETHTOOL_GREGS (bytes) */
 };
 
 #define SOPASS_MAX	6
 /* wake-on-lan settings */
 struct ethtool_wolinfo {
-	u32	cmd;
-	u32	supported;
-	u32	wolopts;
-	u8	sopass[SOPASS_MAX]; /* SecureOn(tm) password */
+	__u32	cmd;
+	__u32	supported;
+	__u32	wolopts;
+	__u8	sopass[SOPASS_MAX]; /* SecureOn(tm) password */
 };
 
 /* for passing single values */
 struct ethtool_value {
-	u32	cmd;
-	u32	data;
+	__u32	cmd;
+	__u32	data;
 };
 
 /* for passing big chunks of data */
 struct ethtool_regs {
-	u32	cmd;
-	u32	version; /* driver-specific, indicates different chips/revs */
-	u32	len; /* bytes */
-	u8	data[0];
+	__u32	cmd;
+	__u32	version; /* driver-specific, indicates different chips/revs */
+	__u32	len; /* bytes */
+	__u8	data[0];
 };
 
 /* for passing EEPROM chunks */
 struct ethtool_eeprom {
-	u32	cmd;
-	u32	magic;
-	u32	offset; /* in bytes */
-	u32	len; /* in bytes */
-	u8	data[0];
+	__u32	cmd;
+	__u32	magic;
+	__u32	offset; /* in bytes */
+	__u32	len; /* in bytes */
+	__u8	data[0];
 };
 
 /* for configuring coalescing parameters of chip */
 struct ethtool_coalesce {
-	u32	cmd;	/* ETHTOOL_{G,S}COALESCE */
+	__u32	cmd;	/* ETHTOOL_{G,S}COALESCE */
 
 	/* How many usecs to delay an RX interrupt after
 	 * a packet arrives.  If 0, only rx_max_coalesced_frames
 	 * is used.
 	 */
-	u32	rx_coalesce_usecs;
+	__u32	rx_coalesce_usecs;
 
 	/* How many packets to delay an RX interrupt after
 	 * a packet arrives.  If 0, only rx_coalesce_usecs is
@@ -94,21 +94,21 @@
 	 * to zero as this would cause RX interrupts to never be
 	 * generated.
 	 */
-	u32	rx_max_coalesced_frames;
+	__u32	rx_max_coalesced_frames;
 
 	/* Same as above two parameters, except that these values
 	 * apply while an IRQ is being serviced by the host.  Not
 	 * all cards support this feature and the values are ignored
 	 * in that case.
 	 */
-	u32	rx_coalesce_usecs_irq;
-	u32	rx_max_coalesced_frames_irq;
+	__u32	rx_coalesce_usecs_irq;
+	__u32	rx_max_coalesced_frames_irq;
 
 	/* How many usecs to delay a TX interrupt after
 	 * a packet is sent.  If 0, only tx_max_coalesced_frames
 	 * is used.
 	 */
-	u32	tx_coalesce_usecs;
+	__u32	tx_coalesce_usecs;
 
 	/* How many packets to delay a TX interrupt after
 	 * a packet is sent.  If 0, only tx_coalesce_usecs is
@@ -116,22 +116,22 @@
 	 * to zero as this would cause TX interrupts to never be
 	 * generated.
 	 */
-	u32	tx_max_coalesced_frames;
+	__u32	tx_max_coalesced_frames;
 
 	/* Same as above two parameters, except that these values
 	 * apply while an IRQ is being serviced by the host.  Not
 	 * all cards support this feature and the values are ignored
 	 * in that case.
 	 */
-	u32	tx_coalesce_usecs_irq;
-	u32	tx_max_coalesced_frames_irq;
+	__u32	tx_coalesce_usecs_irq;
+	__u32	tx_max_coalesced_frames_irq;
 
 	/* How many usecs to delay in-memory statistics
 	 * block updates.  Some drivers do not have an in-memory
 	 * statistic block, and in such cases this value is ignored.
 	 * This value must not be zero.
 	 */
-	u32	stats_block_coalesce_usecs;
+	__u32	stats_block_coalesce_usecs;
 
 	/* Adaptive RX/TX coalescing is an algorithm implemented by
 	 * some drivers to improve latency under low packet rates and
@@ -140,18 +140,18 @@
 	 * not implemented by the driver causes these values to be
 	 * silently ignored.
 	 */
-	u32	use_adaptive_rx_coalesce;
-	u32	use_adaptive_tx_coalesce;
+	__u32	use_adaptive_rx_coalesce;
+	__u32	use_adaptive_tx_coalesce;
 
 	/* When the packet rate (measured in packets per second)
 	 * is below pkt_rate_low, the {rx,tx}_*_low parameters are
 	 * used.
 	 */
-	u32	pkt_rate_low;
-	u32	rx_coalesce_usecs_low;
-	u32	rx_max_coalesced_frames_low;
-	u32	tx_coalesce_usecs_low;
-	u32	tx_max_coalesced_frames_low;
+	__u32	pkt_rate_low;
+	__u32	rx_coalesce_usecs_low;
+	__u32	rx_max_coalesced_frames_low;
+	__u32	tx_coalesce_usecs_low;
+	__u32	tx_max_coalesced_frames_low;
 
 	/* When the packet rate is below pkt_rate_high but above
 	 * pkt_rate_low (both measured in packets per second) the
@@ -162,43 +162,43 @@
 	 * is above pkt_rate_high, the {rx,tx}_*_high parameters are
 	 * used.
 	 */
-	u32	pkt_rate_high;
-	u32	rx_coalesce_usecs_high;
-	u32	rx_max_coalesced_frames_high;
-	u32	tx_coalesce_usecs_high;
-	u32	tx_max_coalesced_frames_high;
+	__u32	pkt_rate_high;
+	__u32	rx_coalesce_usecs_high;
+	__u32	rx_max_coalesced_frames_high;
+	__u32	tx_coalesce_usecs_high;
+	__u32	tx_max_coalesced_frames_high;
 
 	/* How often to do adaptive coalescing packet rate sampling,
 	 * measured in seconds.  Must not be zero.
 	 */
-	u32	rate_sample_interval;
+	__u32	rate_sample_interval;
 };
 
 /* for configuring RX/TX ring parameters */
 struct ethtool_ringparam {
-	u32	cmd;	/* ETHTOOL_{G,S}RINGPARAM */
+	__u32	cmd;	/* ETHTOOL_{G,S}RINGPARAM */
 
 	/* Read only attributes.  These indicate the maximum number
 	 * of pending RX/TX ring entries the driver will allow the
 	 * user to set.
 	 */
-	u32	rx_max_pending;
-	u32	rx_mini_max_pending;
-	u32	rx_jumbo_max_pending;
-	u32	tx_max_pending;
+	__u32	rx_max_pending;
+	__u32	rx_mini_max_pending;
+	__u32	rx_jumbo_max_pending;
+	__u32	tx_max_pending;
 
 	/* Values changeable by the user.  The valid values are
 	 * in the range 1 to the "*_max_pending" counterpart above.
 	 */
-	u32	rx_pending;
-	u32	rx_mini_pending;
-	u32	rx_jumbo_pending;
-	u32	tx_pending;
+	__u32	rx_pending;
+	__u32	rx_mini_pending;
+	__u32	rx_jumbo_pending;
+	__u32	tx_pending;
 };
 
 /* for configuring link flow control parameters */
 struct ethtool_pauseparam {
-	u32	cmd;	/* ETHTOOL_{G,S}PAUSEPARAM */
+	__u32	cmd;	/* ETHTOOL_{G,S}PAUSEPARAM */
 
 	/* If the link is being auto-negotiated (via ethtool_cmd.autoneg
 	 * being true) the user may set 'autonet' here non-zero to have the
@@ -210,9 +210,9 @@
 	 * then {rx,tx}_pause force the driver to use/not-use pause
 	 * flow control.
 	 */
-	u32	autoneg;
-	u32	rx_pause;
-	u32	tx_pause;
+	__u32	autoneg;
+	__u32	rx_pause;
+	__u32	tx_pause;
 };
 
 #define ETH_GSTRING_LEN		32
@@ -223,10 +223,10 @@
 
 /* for passing string sets for data tagging */
 struct ethtool_gstrings {
-	u32	cmd;		/* ETHTOOL_GSTRINGS */
-	u32	string_set;	/* string set id e.c. ETH_SS_TEST, etc*/
-	u32	len;		/* number of strings in the string set */
-	u8	data[0];
+	__u32	cmd;		/* ETHTOOL_GSTRINGS */
+	__u32	string_set;	/* string set id e.c. ETH_SS_TEST, etc*/
+	__u32	len;		/* number of strings in the string set */
+	__u8	data[0];
 };
 
 enum ethtool_test_flags {
@@ -236,28 +236,28 @@
 
 /* for requesting NIC test and getting results*/
 struct ethtool_test {
-	u32	cmd;		/* ETHTOOL_TEST */
-	u32	flags;		/* ETH_TEST_FL_xxx */
-	u32	reserved;
-	u32	len;		/* result length, in number of u64 elements */
-	u64	data[0];
+	__u32	cmd;		/* ETHTOOL_TEST */
+	__u32	flags;		/* ETH_TEST_FL_xxx */
+	__u32	reserved;
+	__u32	len;		/* result length, in number of __u64 elements */
+	__u64	data[0];
 };
 
 /* for dumping NIC-specific statistics */
 struct ethtool_stats {
-	u32	cmd;		/* ETHTOOL_GSTATS */
-	u32	n_stats;	/* number of u64's being returned */
-	u64	data[0];
+	__u32	cmd;		/* ETHTOOL_GSTATS */
+	__u32	n_stats;	/* number of __u64's being returned */
+	__u64	data[0];
 };
 
 struct net_device;
 
 /* Some generic methods drivers may use in their ethtool_ops */
-u32 ethtool_op_get_link(struct net_device *dev);
-u32 ethtool_op_get_tx_csum(struct net_device *dev);
-int ethtool_op_set_tx_csum(struct net_device *dev, u32 data);
-u32 ethtool_op_get_sg(struct net_device *dev);
-int ethtool_op_set_sg(struct net_device *dev, u32 data);
+__u32 ethtool_op_get_link(struct net_device *dev);
+__u32 ethtool_op_get_tx_csum(struct net_device *dev);
+int ethtool_op_set_tx_csum(struct net_device *dev, __u32 data);
+__u32 ethtool_op_get_sg(struct net_device *dev);
+int ethtool_op_set_sg(struct net_device *dev, __u32 data);
 
 /**
  * &ethtool_ops - Alter and report network device settings
@@ -320,31 +320,31 @@
 	void	(*get_regs)(struct net_device *, struct ethtool_regs *, void *);
 	void	(*get_wol)(struct net_device *, struct ethtool_wolinfo *);
 	int	(*set_wol)(struct net_device *, struct ethtool_wolinfo *);
-	u32	(*get_msglevel)(struct net_device *);
-	void	(*set_msglevel)(struct net_device *, u32);
+	__u32	(*get_msglevel)(struct net_device *);
+	void	(*set_msglevel)(struct net_device *, __u32);
 	int	(*nway_reset)(struct net_device *);
-	u32	(*get_link)(struct net_device *);
+	__u32	(*get_link)(struct net_device *);
 	int	(*get_eeprom_len)(struct net_device *);
-	int	(*get_eeprom)(struct net_device *, struct ethtool_eeprom *, u8 *);
-	int	(*set_eeprom)(struct net_device *, struct ethtool_eeprom *, u8 *);
+	int	(*get_eeprom)(struct net_device *, struct ethtool_eeprom *, __u8 *);
+	int	(*set_eeprom)(struct net_device *, struct ethtool_eeprom *, __u8 *);
 	int	(*get_coalesce)(struct net_device *, struct ethtool_coalesce *);
 	int	(*set_coalesce)(struct net_device *, struct ethtool_coalesce *);
 	void	(*get_ringparam)(struct net_device *, struct ethtool_ringparam *);
 	int	(*set_ringparam)(struct net_device *, struct ethtool_ringparam *);
 	void	(*get_pauseparam)(struct net_device *, struct ethtool_pauseparam*);
 	int	(*set_pauseparam)(struct net_device *, struct ethtool_pauseparam*);
-	u32	(*get_rx_csum)(struct net_device *);
-	int	(*set_rx_csum)(struct net_device *, u32);
-	u32	(*get_tx_csum)(struct net_device *);
-	int	(*set_tx_csum)(struct net_device *, u32);
-	u32	(*get_sg)(struct net_device *);
-	int	(*set_sg)(struct net_device *, u32);
+	__u32	(*get_rx_csum)(struct net_device *);
+	int	(*set_rx_csum)(struct net_device *, __u32);
+	__u32	(*get_tx_csum)(struct net_device *);
+	int	(*set_tx_csum)(struct net_device *, __u32);
+	__u32	(*get_sg)(struct net_device *);
+	int	(*set_sg)(struct net_device *, __u32);
 	int	(*self_test_count)(struct net_device *);
-	void	(*self_test)(struct net_device *, struct ethtool_test *, u64 *);
-	void	(*get_strings)(struct net_device *, u32 stringset, u8 *);
-	int	(*phys_id)(struct net_device *, u32);
+	void	(*self_test)(struct net_device *, struct ethtool_test *, __u64 *);
+	void	(*get_strings)(struct net_device *, __u32 stringset, __u8 *);
+	int	(*phys_id)(struct net_device *, __u32);
 	int	(*get_stats_count)(struct net_device *);
-	void	(*get_ethtool_stats)(struct net_device *, struct ethtool_stats *, u64 *);
+	void	(*get_ethtool_stats)(struct net_device *, struct ethtool_stats *, __u64 *);
 };
 
 /* CMDs currently supported */

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ethtool.h should use userspace-accessible types
  2004-03-12  1:54 [PATCH] ethtool.h should use userspace-accessible types Eric Brower
@ 2004-03-12  2:35 ` Tim Hockin
  2004-03-12  4:57   ` Greg KH
  2004-03-12 16:25 ` Eric W. Biederman
  2004-03-12 16:54 ` Jeff Garzik
  2 siblings, 1 reply; 11+ messages in thread
From: Tim Hockin @ 2004-03-12  2:35 UTC (permalink / raw)
  To: Eric Brower; +Cc: linux-kernel

On Thu, Mar 11, 2004 at 05:54:48PM -0800, Eric Brower wrote:
> Attached is a patch to 2.4's ethtool.h to use appropriate, 
> userspace-accessible data types (__u8 and friends, rather than u8 and 
> friends).

if we *know* the width of them, why don't we just use C99 standard types in
*both* places?  I've never quite grokked why we need u8 and __u8 and all the
variants, when we now have uint8_t.  I mean, at least it's standardized.

Anyone have a logical answer?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ethtool.h should use userspace-accessible types
  2004-03-12  2:35 ` Tim Hockin
@ 2004-03-12  4:57   ` Greg KH
  2004-03-12  9:19     ` Tim Hockin
  0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2004-03-12  4:57 UTC (permalink / raw)
  To: Tim Hockin; +Cc: Eric Brower, linux-kernel

On Thu, Mar 11, 2004 at 06:35:51PM -0800, Tim Hockin wrote:
> On Thu, Mar 11, 2004 at 05:54:48PM -0800, Eric Brower wrote:
> > Attached is a patch to 2.4's ethtool.h to use appropriate, 
> > userspace-accessible data types (__u8 and friends, rather than u8 and 
> > friends).
> 
> if we *know* the width of them, why don't we just use C99 standard types in
> *both* places?  I've never quite grokked why we need u8 and __u8 and all the
> variants, when we now have uint8_t.  I mean, at least it's standardized.
> 
> Anyone have a logical answer?

Yes.
	u8 means an unsigned 8 bit variable within the kernel.
	__u8 means an unsigned 8 bit variable both within the kernel and
	in userspace.  Use the __ forms when describing data structures
	or variables that cross the userspace/kernelspace boundry in
	order to get everything correct.

Becides, something like u8 is a zillion times saner than uint8_t, don't
you think?  :)

I remember saying that I would document this better a long time ago, as
it comes up every other month or so.  Sorry for not doing it yet...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ethtool.h should use userspace-accessible types
  2004-03-12  4:57   ` Greg KH
@ 2004-03-12  9:19     ` Tim Hockin
  0 siblings, 0 replies; 11+ messages in thread
From: Tim Hockin @ 2004-03-12  9:19 UTC (permalink / raw)
  To: Greg KH; +Cc: Eric Brower, linux-kernel

On Thu, Mar 11, 2004 at 08:57:56PM -0800, Greg KH wrote:
> 	u8 means an unsigned 8 bit variable within the kernel.
> 	__u8 means an unsigned 8 bit variable both within the kernel and
> 	in userspace.  Use the __ forms when describing data structures
> 	or variables that cross the userspace/kernelspace boundry in
> 	order to get everything correct.

The only problem is that this is a RETARDED precedent. __foo means the 
"internal" version of foo. Within the kernel __foo often means the version
of foo without "the lock".

POSIX defines any type beginning with _ as part of the implementation.
IMHO, no userspace app should *ever* need a type that starts with _ or __.
It's just dumb.  If the width matters, define it as a width specific type.
If it doesn't define it as an int/short/whatever.  The fact that it isn't
obvious what is or is not kernel-only is an entirely separate issue.

> Becides, something like u8 is a zillion times saner than uint8_t, don't
> you think?  :)

As much as uint8_t is a pain in the ass to type, at least it is clear and
obvious and standard, don't you think? :)


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ethtool.h should use userspace-accessible types
  2004-03-12  1:54 [PATCH] ethtool.h should use userspace-accessible types Eric Brower
  2004-03-12  2:35 ` Tim Hockin
@ 2004-03-12 16:25 ` Eric W. Biederman
  2004-03-12 16:54   ` Jeff Garzik
  2004-03-12 16:54 ` Jeff Garzik
  2 siblings, 1 reply; 11+ messages in thread
From: Eric W. Biederman @ 2004-03-12 16:25 UTC (permalink / raw)
  To: linux-kernel; +Cc: Eric Brower

Eric Brower <ebrower@usa.net> writes:

> Attached is a patch to 2.4's ethtool.h to use appropriate, userspace-accessible
> data types (__u8 and friends, rather than u8 and friends).

Why there is no #ifdef __KERNEL__ in this header to make it userspace
safe.

> I've also posted a patch against ethtool-1.8 to the sf.net gkernel site that
> cleans-up ethtool-copy.h and the rest in the same manner.

In user space the types from stdint.h likely should be used.  So __u8
and friends looks wrong there as well.

Eric

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ethtool.h should use userspace-accessible types
  2004-03-12 16:25 ` Eric W. Biederman
@ 2004-03-12 16:54   ` Jeff Garzik
  2004-03-12 17:06     ` Eric W. Biederman
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Garzik @ 2004-03-12 16:54 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: linux-kernel, Eric Brower

Eric W. Biederman wrote:
> Eric Brower <ebrower@usa.net> writes:
> 
> 
>>Attached is a patch to 2.4's ethtool.h to use appropriate, userspace-accessible
>>data types (__u8 and friends, rather than u8 and friends).
> 
> 
> Why there is no #ifdef __KERNEL__ in this header to make it userspace
> safe.


Because it's not needed.

	Jeff




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ethtool.h should use userspace-accessible types
  2004-03-12  1:54 [PATCH] ethtool.h should use userspace-accessible types Eric Brower
  2004-03-12  2:35 ` Tim Hockin
  2004-03-12 16:25 ` Eric W. Biederman
@ 2004-03-12 16:54 ` Jeff Garzik
  2 siblings, 0 replies; 11+ messages in thread
From: Jeff Garzik @ 2004-03-12 16:54 UTC (permalink / raw)
  To: Eric Brower; +Cc: linux-kernel

Eric Brower wrote:
> Attached is a patch to 2.4's ethtool.h to use appropriate, 
> userspace-accessible data types (__u8 and friends, rather than u8 and 
> friends).

I'm happy with u8/u16/u32, so it stays that way :)

	Jeff




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ethtool.h should use userspace-accessible types
  2004-03-12 16:54   ` Jeff Garzik
@ 2004-03-12 17:06     ` Eric W. Biederman
  2004-03-12 21:48       ` Jonathan Lundell
  0 siblings, 1 reply; 11+ messages in thread
From: Eric W. Biederman @ 2004-03-12 17:06 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: linux-kernel

Jeff Garzik <jgarzik@pobox.com> writes:

> Eric W. Biederman wrote:
> > Eric Brower <ebrower@usa.net> writes:
> >
> >>Attached is a patch to 2.4's ethtool.h to use appropriate,
> userspace-accessible
> 
> >>data types (__u8 and friends, rather than u8 and friends).
> > Why there is no #ifdef __KERNEL__ in this header to make it userspace
> > safe.
> 
> 
> Because it's not needed.

I think we are in agreement.

My intent was to say:  Why change the types when there is no #ifdef
__KERNEL__ in the header.  With no #ifdef __KERNEL__ it exports
definitions that are private to the kernel making it not safe for
userspace to use.  With kernel private definitions in there it will
generate name space pollution if included by user space.

Eric

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ethtool.h should use userspace-accessible types
  2004-03-12 17:06     ` Eric W. Biederman
@ 2004-03-12 21:48       ` Jonathan Lundell
  2004-03-14  3:25         ` Matt Mackall
  0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Lundell @ 2004-03-12 21:48 UTC (permalink / raw)
  To: linux-kernel

At 10:06 AM -0700 3/12/04, Eric W. Biederman wrote:
>My intent was to say:  Why change the types when there is no #ifdef
>__KERNEL__ in the header.  With no #ifdef __KERNEL__ it exports
>definitions that are private to the kernel making it not safe for
>userspace to use.  With kernel private definitions in there it will
>generate name space pollution if included by user space.

Presumably because it *is* included by userspace, because it defines 
the interface between the kernel and userspace; of course userspace 
will (does) include it.

-- 
/Jonathan Lundell.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ethtool.h should use userspace-accessible types
  2004-03-12 21:48       ` Jonathan Lundell
@ 2004-03-14  3:25         ` Matt Mackall
  2004-03-15 21:24           ` Jonathan Lundell
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Mackall @ 2004-03-14  3:25 UTC (permalink / raw)
  To: Jonathan Lundell; +Cc: linux-kernel

On Fri, Mar 12, 2004 at 01:48:54PM -0800, Jonathan Lundell wrote:
> At 10:06 AM -0700 3/12/04, Eric W. Biederman wrote:
> >My intent was to say:  Why change the types when there is no #ifdef
> >__KERNEL__ in the header.  With no #ifdef __KERNEL__ it exports
> >definitions that are private to the kernel making it not safe for
> >userspace to use.  With kernel private definitions in there it will
> >generate name space pollution if included by user space.
> 
> Presumably because it *is* included by userspace, because it defines 
> the interface between the kernel and userspace; of course userspace 
> will (does) include it.

Well that's a bug. You don't include kernel headers in userspace.
Doing so has been deprecated for 8 years and 3 major releases.

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ethtool.h should use userspace-accessible types
  2004-03-14  3:25         ` Matt Mackall
@ 2004-03-15 21:24           ` Jonathan Lundell
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Lundell @ 2004-03-15 21:24 UTC (permalink / raw)
  To: linux-kernel

At 9:25 PM -0600 3/13/04, Matt Mackall wrote:
>On Fri, Mar 12, 2004 at 01:48:54PM -0800, Jonathan Lundell wrote:
>>  At 10:06 AM -0700 3/12/04, Eric W. Biederman wrote:
>>  >My intent was to say:  Why change the types when there is no #ifdef
>>  >__KERNEL__ in the header.  With no #ifdef __KERNEL__ it exports
>>  >definitions that are private to the kernel making it not safe for
>>  >userspace to use.  With kernel private definitions in there it will
>>  >generate name space pollution if included by user space.
>>
>>  Presumably because it *is* included by userspace, because it defines
>>  the interface between the kernel and userspace; of course userspace
>>  will (does) include it.
>
>Well that's a bug. You don't include kernel headers in userspace.
>Doing so has been deprecated for 8 years and 3 major releases.

Then it's a bug for the ethtool maintainer. ethtool.c #includes 
ethtool-copy.h, a renamed copy of the kernel header file.

I'm familiar with the deprecation and continue to be bewildered by 
it. To take this particular example, ethtool.h defines an interface 
between a kernel facility and a userspace client. What's the 
non-deprecated method of sharing the interface definition?
-- 
/Jonathan Lundell.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2004-03-15 21:24 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-03-12  1:54 [PATCH] ethtool.h should use userspace-accessible types Eric Brower
2004-03-12  2:35 ` Tim Hockin
2004-03-12  4:57   ` Greg KH
2004-03-12  9:19     ` Tim Hockin
2004-03-12 16:25 ` Eric W. Biederman
2004-03-12 16:54   ` Jeff Garzik
2004-03-12 17:06     ` Eric W. Biederman
2004-03-12 21:48       ` Jonathan Lundell
2004-03-14  3:25         ` Matt Mackall
2004-03-15 21:24           ` Jonathan Lundell
2004-03-12 16:54 ` Jeff Garzik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).