linux-kernel - Re: regulator_get_optional() no longer returning NULL?

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: <20170203112001.m5nwfrw7ickum3r7@sirena.org.uk>
Date:   Fri, 3 Feb 2017 12:20:01 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     Thierry Reding <treding@...dia.com>,
        Jelle van der Waa <jelle@...aa.nl>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: regulator_get_optional() no longer returning NULL?

On Wed, Feb 01, 2017 at 02:23:31PM -0800, Dmitry Torokhov wrote:

> It appears that [devm_]regulator_get_optional() and
> [devm_]gpiod_get_optional() behave irritatingly differently. While the
> latter returns NULL for non-existing GPIOs, the former started returning
> -ENODEV instead of NULL, starting with commit below.

> Why did we do that? It is much more convenient to write:

As the commit description for that commit says it's fixing a bug with us
not returning the error code we get from regulator_dev_lookup().

> I.e. it is nice to treat *all* codes returned by
> devm_regulator_get_optional() as fatal and NULL as special instead of
> vetting by hand (and having chance that list of vetted codes will bit
> rot).

There's no real risk of bitrot I can see here, the only thing something
using optional regulator is looking for is that the regulator does not
exist and we have the specific error code -ENODEV for that.

> Can we please revert this patch?

That won't do what you want so I don't know why you're asking for it -
the default value of regulator is ERR_PTR(-EPROBE_DEFER) so if we skip
assigning the actual return code you'll get that and not NULL.  I don't
recall that behaviour ever working, it's not what's documented so it
doesn't look like something broke here.  If you wanted to implement it
you'd need to add new code to do it (eg, squash down -ENODEV in
_get_optional()).

However I am a bit dubious about doing that as I'm not thrilled with
adding in more things for people to check.  Even if you check for NULL
with an optional regulator you'd still need to also go and check that
you didn't get an error code like -EPROBE_DEFER, you'd only get a NULL
in cases where the regulator does not exist.  Given that I'm not sure
that it's actually simplifying things.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)