netdev - Re: [PATCH RFC net-next 3/7] net: dsa: use fwnode_get_phy

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]

Message-ID: <ZBxpeLOmTMzqVTRV@smile.fi.intel.com>
Date:   Thu, 23 Mar 2023 17:00:08 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Daniel Scally <djrscally@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Jakub Kicinski <kuba@...nel.org>, linux-acpi@...r.kernel.org,
        netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Vladimir Oltean <olteanv@...il.com>
Subject: Re: [PATCH RFC net-next 3/7] net: dsa: use fwnode_get_phy_mode() to
 get phy interface mode

On Thu, Mar 23, 2023 at 02:49:01PM +0000, Russell King (Oracle) wrote:
> On Thu, Mar 23, 2023 at 04:38:29PM +0200, Andy Shevchenko wrote:
> > On Thu, Mar 23, 2023 at 02:31:04PM +0000, Russell King (Oracle) wrote:
> > > On Thu, Mar 23, 2023 at 04:03:05PM +0200, Andy Shevchenko wrote:
> > > > On Wed, Mar 22, 2023 at 12:00:06PM +0000, Russell King (Oracle) wrote:

...

> > > > > +	struct fwnode_handle *fwnode;
> > > > 
> > > > > +	fwnode = of_fwnode_handle(dp->dn);
> > > > 
> > > > 	const struct fwnode_handle *fwnode = of_fwnode_handle(dp->dn);
> > > > 
> > > > ?
> > > 
> > > Why const?
> > 
> > Do you modify its content on the fly?
> 
> Do you want to litter code with casts to get rid of the const?
> 
> > For fwnode as a basic object type we want to reduce the scope of the possible
> > modifications. If you don't modify and APIs you call do not require non-const
> > object, use const for fwnode.
> 
> Let's start here. We pass this fwnode to fwnode_get_phy_mode():
> 
> include/linux/property.h:int fwnode_get_phy_mode(struct fwnode_handle *fwnode);
> 
> Does fwnode_get_phy_mode() alter the contents of the fwnode? Probably
> not, but it doesn't take a const pointer. Therefore, to declare my
> fwnode as const, I'd need to cast the const-ness away before calling
> this.

So, fix the fwnode_get_phy_mode(). Is it a problem?

> Then there's phylink_create(). Same problem.

So, fix that. Is it a problem?

> So NAK to this const - until such time that we have a concerted effort
> to making functions we call which do not modify the "fwnode" argument
> constify that argument. Otherwise it's just rediculously crazy to
> declare a variable const only to then litter the code with casts to get
> rid of it at every call site.
> 
> Please do a bit of research before making suggestions. Thanks.

So, MAK to your patch. You can fix that, and you know that.

P.S. Please, move that phy thingy away from property.h, it doesn't belong
there.

-- 
With Best Regards,
Andy Shevchenko