netdev - Re: [RFC PATCH 11/16] net: dsa: Refactor selection of tag ops into a function

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20160527201143.GB23212@lunn.ch>
Date:	Fri, 27 May 2016 22:11:43 +0200
From:	Andrew Lunn <andrew@...n.ch>
To:	Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Cc:	netdev <netdev@...r.kernel.org>,
	Florian Fainelli <f.fainelli@...il.com>,
	John Crispin <john@...ozen.org>, Bryan.Whitehead@...rochip.com
Subject: Re: [RFC PATCH 11/16] net: dsa: Refactor selection of tag ops into a
 function

On Fri, May 27, 2016 at 03:35:57PM -0400, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn <andrew@...n.ch> writes:
> 
> > @@ -26,6 +26,7 @@ enum dsa_tag_protocol {
> >  	DSA_TAG_PROTO_TRAILER,
> >  	DSA_TAG_PROTO_EDSA,
> >  	DSA_TAG_PROTO_BRCM,
> > +	_DSA_TAG_LAST,
> >  };
> 
> I would avoid _ prefixed functions or symbols, we've seen in mv88e6xxx
> that it doesn't make the code really readable.

The point is to make is special, so that people look at it, wonder why
it is special, and don't make the stupid mistakes of adding a new
protocol after it.

However, if you don't like the _, we can just add a comment
/* MUST BE LAST */

> I don't see the need to add a dsa_device_ops array. I'd keep the switch
> case on tag_protocol within this new dsa_resolve_tag_protocol function,
> to make it more readable (no value checking necessary).

I don't see one way being better than the other. I see it as a
personal preference. If you don't like it, i won't NACK a patch
submitted after it has been accepted which changes it to your personal
preference.

      Andrew