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

Date:   Tue, 7 Mar 2023 10:13:26 -0800
From:   Grant Grundler <grundler@...omium.org>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     Grant Grundler <grundler@...omium.org>,
        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 Tue, Mar 7, 2023 at 2:29 AM Lukas Wunner <lukas@...ner.de> wrote:
>
> 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.

Ok - will do.

> 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.

Sure. No problem. The code I wrote seemed easier to read but I'm
familiar with the convention and have no objection to that.

I'll add the Fixes: tag as well (thanks for confirming Oleksij!)

cheers,
grant

>
> Thanks,
>
> Lukas