On Fri, Jul 11, 2003 at 12:51:24PM -0700, Ben Greear wrote:
> >3) The func ptrs _count() are totally bogus. We have an unconditional
> >indirect reference to a function call which does nothing but return a
> >driver constant.
> >I personally think that having ethtool_ops members manually calling
> >the ->get_drvinfo hook is a _lot_ cleaner than 10,000 foo_count hooks.
> >3.a) Further, we will inevitably be adding more counts in the future.
> >If we wanted to be truly expandable, and you really don't like the
> >counts being in struct ethtool_gdrvinfo, then create a struct
> >ethtool_counts that puts all the constants in one place.
> Suppose we do this, how will we make a user-space app that tries to
> read this be backwards/forwards compatible?
It's trivial to return the existing values in the gdrvinfo struct in
addition to a new ethtool_count struct. Full ABI compat is maintained.
> >4) I don't see why ethtool.h suddenly needs to include linux/types.h,
> >when it hasn't needed it in all this time until now.
> If we're changing lots of stuff...it would be nice to change the u32
> etc to something that user-space can easily handle, as ethtool.h is
> (for better or worse) being included from user-space. Minor issue
> again as it can be dealt with via type-defs etc.
ethtool.h isn't included in the "lots of stuff" that is changing
:) As you see from Matthew's patch, all changes to ethtool.h are
Regardless, addressing your point, I consider ethtool.h a
kernel-internal header, that's why it uses internal kernel types.
Anybody who copies it to userspace must deal with that. It is _not_
intended to be #included directly from userspace. ethtool (the userland
program) purposefully does its own typedefs and stuff.