linux-kernel - Re: [PATCH v2 2/3] gpio: gpio-regmap: Use devm_add

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]

Date:   Fri, 21 May 2021 10:30:18 +0000
From:   "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To:     "michael@...le.cc" <michael@...le.cc>,
        "bjorn@...k.no" <bjorn@...k.no>
CC:     linux-power <linux-power@...rohmeurope.com>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "bgolaszewski@...libre.com" <bgolaszewski@...libre.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>
Subject: Re: [PATCH v2 2/3] gpio: gpio-regmap: Use devm_add_action()


On Fri, 2021-05-21 at 10:38 +0200, Bjørn Mork wrote:
> Michael Walle <michael@...le.cc> writes:
> 
> > Am 2021-05-21 08:28, schrieb Matti Vaittinen:
> > > Slightly simplify the devm_gpio_regmap_register() by using the
> > > devm_add_action().
> > 
> > Hm, nice, but what bothers me a bit is that no other subsystem
> > does it that way, eg. hwmon/hwmon.c or watchdog/watchdog_core.c.
> > They also store just one pointer, thus could be simplified in the
> > same way. What I don't know is if devm_add_action() was intended
> > to be used this way. So I can't say much for this patch ;)
> 
> There are some examples.  Like:
> 
> int devm_i2c_add_adapter(struct device *dev, struct i2c_adapter
> *adapter)
> {
>         int ret;
> 
>         ret = i2c_add_adapter(adapter);
>         if (ret)
>                 return ret;
> 
>         return devm_add_action_or_reset(dev, devm_i2c_del_adapter,
> adapter);
> }
> 
> 
> You should probably use the devm_add_action_or_reset() wrapper here
> too,
> catching the unlikely devm_add_action() alloc failure.
> 

I was thinking of it but as the gpio registration succeeded I was
thinking that we could go on with it - (which means we can proceed but
the gpio is never released.)

I am not sure how much difference it makes in the case of small alloc
failure ;)

But as it seems I am in any case re-spinning this I can change this to
the devm_add_action_or_reset() and fail the gpio_regmap registration if
alloc fails.

Best Regards
	Matti Vaittinen