netdev - Re: [PATCH] net: asix: fix modprobe "sysfs: cannot create duplicate filename"

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-next>] [day] [month] [year] [list]

Date:   Tue, 7 Mar 2023 11:29:31 +0100
From:   Lukas Wunner <lukas@...ner.de>
To:     Grant Grundler <grundler@...omium.org>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Oleksij Rempel <linux@...pel-privat.de>,
        Pavel Skripkin <paskripkin@...il.com>,
        Anton Lundin <glance@....umu.se>,
        Eizan Miyamoto <eizan@...omium.org>,
        Jakub Kicinski <kuba@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] net: asix: fix modprobe "sysfs: cannot create duplicate
 filename"

On Mon, Mar 06, 2023 at 10:10:09PM -0800, Grant Grundler wrote:
> On Mon, Mar 6, 2023 at 7:46???PM Florian Fainelli <f.fainelli@...il.com> wrote:
> > On 3/6/2023 4:50 PM, Grant Grundler wrote:
> > > +     priv->phydev = mdiobus_get_phy(priv->mdio, priv->phy_addr);
> > > +     if (priv->phydev)
> > > +             return 0;
> >
> > This was in ax88772_init_phy() before, why is this being moved here now?
> 
> Because other drivers I looked at (e.g. tg3 and r8169) do all the mdiobus_*
> calls in one function and I wanted to have some "symmetry"
> with ax88772_release_mdio() function I added below.

I'd suggest moving this cleanup to a separate commit so that you keep
the fix itself as small as possible and thus minimize the potential of
introducing regressions in stable kernels that will receive the fix.

Also, per convention please use the if-clause to catch the error case,
not the success case.  It doesn't matter if you need two or three more
lines, readability is more important IMO.

Thanks,

Lukas