linux-kernel - Re: [RFC PATCH 01/10] mmc: core: improve API to make clear mmc_hw

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: <CAPDyKFo5aO-s13sP4MyjZgP-w+1Bmd59a-o+t3pVA+NgKoCHGg@mail.gmail.com>
Date:   Wed, 6 Apr 2022 15:53:19 +0200
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        linux-mmc@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 01/10] mmc: core: improve API to make clear
 mmc_hw_reset is for cards

On Wed, 6 Apr 2022 at 09:58, Wolfram Sang
<wsa+renesas@...g-engineering.com> wrote:
>
> Hi Ulf,
>
> > > To make it unambiguous that mmc_hw_reset() is for cards and not for
> > > controllers, we a) add 'card' to the function name and b) make the
> > > function argument mmc_card instead of mmc_host. A fallback is provided
> > > until all users are converted.
> >
> > In my opinion, I think b) is sufficient and would be a nice improvement.
>
> If you say so... but I do wonder why we can't be super clear with the
> function name alone without the function argument as an additional
> source of information? Kernel hacking is complicated enough.
>
> > In this regard, I suggest we make one big cross-subsystem patch (the
> > smallest change as possible) then I can pick it up and send it for the
> > v5.18-rc2.
>
> Ok, I can prepare this.

Great!

>
> > > -static void mmc_hw_reset_for_init(struct mmc_host *host)
> > > +/* we can't use mmc_card as a parameter, it is not populated yet */
> >
> > Please drop this. The function is internal/static and at least to me,
> > rather self-explanatory.
>
> All other ?w_reset() functions have a card as a parameter. For people
> trying to get into the MMC core, this comment might be helpful to
> understand the anomaly? I know that you as the maintainer do know this
> by heart, this comment is meant for people learning the stuff.

I understand your point, however I don't think it makes much sense to
try to clarify the names on mmc_hw|sw_reset() alone. There are simply
lots of other functions that then would need to be changed too.
Otherwise we would just end up with having even more in-consistency in
function namings. To me, that's even worse.

>
> All the best,
>
>    Wolfram

Kind regards
Uffe