netdev - Re: [net-next PATCH 1/2] net: ethtool: add define for link speed mode number

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 13 Dec 2023 21:15:27 +0100
From: Christian Marangi <ansuelsmth@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Vladimir Oltean <vladimir.oltean@....com>,
	Vincent Mailhol <mailhol.vincent@...adoo.fr>,
	Kees Cook <keescook@...omium.org>,
	Piergiorgio Beruto <piergiorgio.beruto@...il.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [net-next PATCH 1/2] net: ethtool: add define for link speed
 mode number

On Wed, Dec 13, 2023 at 08:10:42PM +0000, Russell King (Oracle) wrote:
> NAK.
> 
> You *clearly* didn't look before you leaped.
> 
> On Wed, Dec 13, 2023 at 07:15:53PM +0100, Christian Marangi wrote:
> > +enum ethtool_link_speeds {
> > +	SPEED_10 = 0,
> > +	SPEED_100,
> > +	SPEED_1000,
> ...
> 
> and from the context immediately below, included in your patch:
> >  #define SPEED_10		10
>            ^^^^^^^^
> >  #define SPEED_100		100
>            ^^^^^^^^^
> >  #define SPEED_1000		1000
>            ^^^^^^^^^^
> 
> Your enumerated values will be overridden by the preprocessor
> definitions.
> 
> Moreover, SPEED_xxx is an already taken namespace and part of the UAPI,
> and thus can _not_ be changed. Convention is that SPEED_x will be
> defined as the numeric speed.
>

Well yes that is the idea of having the enum to count them and then redefining
them to the correct value. (wasn't trying to introduce new define for
the speed and trying to assign incremental values)

Any idea how to handle this without the enum - redefine thing?

Was trying to find a more automated way than defining the raw number of
the current modes. (but maybe this is not that bad? since on adding more
modes, other values has to be changed so it would be just another value
to document in the comment)

-- 
	Ansuel