Re: [PATCH] Proposed removal of IS_ERR_OR_NULL() (was: Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface)
On Wed, Jan 9, 2013 at 4:04 PM, Russell King - ARM Linux wrote: > So, it seems there's some concensus building here, and it seems that > I've become the chosen victi^wvolunteer for this. So, here's a patch. > It's missing a Guns-supplied-by: tag though. > > From: Russell King > Subject: Mark IS_ERR_OR_NULL() deprecated Acked-by: Linus Walleij Following the tradition of computer science an alternate title of the patch could be "IS_ERR_OR_NULL() considered harmful". Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface
On Wed, Jan 09, 2013 at 11:10:55AM +, Russell King - ARM Linux wrote:
[...]
> err = gpio_request(en_vdd_1v05, "EN_VDD_1V05");
> if (err) {
> pr_err("%s: gpio_request failed: %d\n", __func__, err);
> return err;
> }
>
> gpio_direction_output(en_vdd_1v05, 1);
>
> regulator = regulator_get(NULL, "vdd_ldo0,vddio_pex_clk");
> if (IS_ERR_OR_NULL(regulator)) {
> pr_err("%s: regulator_get failed: %d\n", __func__,
>(int)PTR_ERR(regulator));
> goto err_reg;
> }
> ...
> err_reg:
> gpio_free(en_vdd_1v05);
>
> return err;
> }
>
> So 'err' here is never set. when IS_ERR_OR_NULL evaluates true.
> Setting 'err' to PTR_ERR(regulator) is not correct because a NULL return
> sets 'err' to zero.
FWIW, this one is fixed in the Tegra PCIe series I posted yesterday.
Thierry
pgpHHHlYWJ49k.pgp
Description: PGP signature
Re: [PATCH] Proposed removal of IS_ERR_OR_NULL() (was: Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface)
* Russell King - ARM Linux [130109 09:15]: > On Wed, Jan 09, 2013 at 04:21:45PM +, Russell King - ARM Linux wrote: > > On Wed, Jan 09, 2013 at 11:09:23AM -0500, Nicolas Pitre wrote: > > > On Wed, 9 Jan 2013, Russell King - ARM Linux wrote: > > > > > > > On Wed, Jan 09, 2013 at 10:27:53AM -0500, Nicolas Pitre wrote: > > > > > Anyone with good coccinelle skills around to deal with the users? > > > > > > > > I'm not sure that's a solution. > > > > > > Well, I was thinking that coccinelle could handle the majority of the > > > 354 users when the "fix" is obvious enough to be automated. > > > > > > That said, if we want people to fix their code, it is probably necessary > > > to merge your patch right away so the warnings are actually being seen, > > > and revert it right before the final v3.8 release if the remaining > > > warnings are still too numerous. Repeat with next cycle. > > > > Well, this is what I currently have for arch/arm thus far, and ooh look, > > we save some lines of code too. > > Actually, the OMAP dmtimer.c code needs a few more fixes while we're > reviewing this stuff... This latest patch leaves three files in > arch/arm still using IS_ERR_OR_NULL() as they do seem to want to > legitimately check for error pointers _and_ NULL. > > I'm restricting the patch below to just those cases where it's wrong > in arch/arm. > > The omap_device.c changes probably also deserve some explanation. As > far as I can see, if we have an 'od' then 'od->pdev' _must_ already > be valid (pdev is passed into omap_device_alloc() which creates the > 'od' - and the passed pdev better be checked _before_ it's passed in.) > It also appears that oh->od will _never_ be an error pointer value - > apart from tracing the paths creating that, there is some evidence for > this with tests elsewhere just for NULL here. And lastly > omap_hwmod_lookup() is documented to _only_ return NULL on error, and > review of it confirms that. Looks correct to me. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Proposed removal of IS_ERR_OR_NULL() (was: Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface)
On Wed, Jan 09, 2013 at 04:21:45PM +, Russell King - ARM Linux wrote:
> On Wed, Jan 09, 2013 at 11:09:23AM -0500, Nicolas Pitre wrote:
> > On Wed, 9 Jan 2013, Russell King - ARM Linux wrote:
> >
> > > On Wed, Jan 09, 2013 at 10:27:53AM -0500, Nicolas Pitre wrote:
> > > > Anyone with good coccinelle skills around to deal with the users?
> > >
> > > I'm not sure that's a solution.
> >
> > Well, I was thinking that coccinelle could handle the majority of the
> > 354 users when the "fix" is obvious enough to be automated.
> >
> > That said, if we want people to fix their code, it is probably necessary
> > to merge your patch right away so the warnings are actually being seen,
> > and revert it right before the final v3.8 release if the remaining
> > warnings are still too numerous. Repeat with next cycle.
>
> Well, this is what I currently have for arch/arm thus far, and ooh look,
> we save some lines of code too.
Actually, the OMAP dmtimer.c code needs a few more fixes while we're
reviewing this stuff... This latest patch leaves three files in
arch/arm still using IS_ERR_OR_NULL() as they do seem to want to
legitimately check for error pointers _and_ NULL.
I'm restricting the patch below to just those cases where it's wrong
in arch/arm.
The omap_device.c changes probably also deserve some explanation. As
far as I can see, if we have an 'od' then 'od->pdev' _must_ already
be valid (pdev is passed into omap_device_alloc() which creates the
'od' - and the passed pdev better be checked _before_ it's passed in.)
It also appears that oh->od will _never_ be an error pointer value -
apart from tracing the paths creating that, there is some evidence for
this with tests elsewhere just for NULL here. And lastly
omap_hwmod_lookup() is documented to _only_ return NULL on error, and
review of it confirms that.
Other stuff.
- soc_device_register() returns error-pointer values only. Nothing
allows it to return NULL.
- soc_device_to_device() merely translates the type of the pointer
and has no failure value - plus code paths make use of this pointer
outside of the IS_ERR_OR_NULL() check, so it better be valid.
- debugfs interfaces return NULL on error. Everything else should be
considered valid. (Yes, it returns ERR_PTR(-ENODEV) when it's
unconfigured, but that's to make it _look_ like a valid pointer to
prevent these paths from erroring out unnecessarily.)
- pwrdm_can_ever_lose_context() is only ever calls from the GPIO code,
and the source of the passed pointer is defined to only ever be a
valid pointer or NULL.
- regulator API, errors are valid error pointers. NULL means regulator
API not available. Whether it's right that board-harmony-pcie.c
should not proceed without the regulator API is debatable, but the
current code is just fscked.
- clk_get_sys/clk_get - I've been over that many times, I don't need to
restate it yet again.
arch/arm/mach-integrator/integrator_ap.c |6 ++
arch/arm/mach-integrator/integrator_cp.c |7 ++-
arch/arm/mach-omap2/omap_device.c| 11 ---
arch/arm/mach-omap2/pm-debug.c |6 +++---
arch/arm/mach-omap2/powerdomain.c|2 +-
arch/arm/mach-tegra/board-harmony-pcie.c |6 +++---
arch/arm/mach-tegra/tegra2_emc.c |2 +-
arch/arm/mach-ux500/cpu.c|5 ++---
arch/arm/plat-omap/dmtimer.c | 10 +-
9 files changed, 23 insertions(+), 32 deletions(-)
diff --git a/arch/arm/mach-integrator/integrator_ap.c
b/arch/arm/mach-integrator/integrator_ap.c
index 11e2a41..61225e1 100644
--- a/arch/arm/mach-integrator/integrator_ap.c
+++ b/arch/arm/mach-integrator/integrator_ap.c
@@ -540,16 +540,14 @@ static void __init ap_init_of(void)
'A' + (ap_sc_id & 0x0f));
soc_dev = soc_device_register(soc_dev_attr);
- if (IS_ERR_OR_NULL(soc_dev)) {
+ if (IS_ERR(soc_dev)) {
kfree(soc_dev_attr->revision);
kfree(soc_dev_attr);
return;
}
parent = soc_device_to_device(soc_dev);
-
- if (!IS_ERR_OR_NULL(parent))
- integrator_init_sysfs(parent, ap_sc_id);
+ integrator_init_sysfs(parent, ap_sc_id);
of_platform_populate(root, of_default_bus_match_table,
ap_auxdata_lookup, parent);
diff --git a/arch/arm/mach-integrator/integrator_cp.c
b/arch/arm/mach-integrator/integrator_cp.c
index 7322838..6016189 100644
--- a/arch/arm/mach-integrator/integrator_cp.c
+++ b/arch/arm/mach-integrator/integrator_cp.c
@@ -364,17 +364,14 @@ static void __init intcp_init_of(void)
'A' + (intcp_sc_id & 0x0f));
soc_dev = soc_device_register(soc_dev_attr);
- if (IS_ERR_OR_NULL(soc_dev)) {
+ if (IS_ERR(soc_dev)) {
kfree(soc_dev_attr->revision);
kfree(soc_dev_attr);
return;
}
parent = soc_
Re: [PATCH] Proposed removal of IS_ERR_OR_NULL() (was: Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface)
On Wed, Jan 09, 2013 at 11:09:23AM -0500, Nicolas Pitre wrote:
> On Wed, 9 Jan 2013, Russell King - ARM Linux wrote:
>
> > On Wed, Jan 09, 2013 at 10:27:53AM -0500, Nicolas Pitre wrote:
> > > Anyone with good coccinelle skills around to deal with the users?
> >
> > I'm not sure that's a solution.
>
> Well, I was thinking that coccinelle could handle the majority of the
> 354 users when the "fix" is obvious enough to be automated.
>
> That said, if we want people to fix their code, it is probably necessary
> to merge your patch right away so the warnings are actually being seen,
> and revert it right before the final v3.8 release if the remaining
> warnings are still too numerous. Repeat with next cycle.
Well, this is what I currently have for arch/arm thus far, and ooh look,
we save some lines of code too.
arch/arm/mach-integrator/integrator_ap.c |4 +---
arch/arm/mach-integrator/integrator_cp.c |5 +
arch/arm/mach-omap2/omap_device.c| 11 ---
arch/arm/mach-omap2/pm-debug.c |6 +++---
arch/arm/mach-omap2/powerdomain.c|2 +-
arch/arm/mach-tegra/board-harmony-pcie.c |6 +++---
arch/arm/mach-tegra/tegra2_emc.c |2 +-
arch/arm/mach-ux500/cpu.c|3 +--
arch/arm/plat-omap/dmtimer.c |4 ++--
9 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/arch/arm/mach-integrator/integrator_ap.c
b/arch/arm/mach-integrator/integrator_ap.c
index 11e2a41..251f54f 100644
--- a/arch/arm/mach-integrator/integrator_ap.c
+++ b/arch/arm/mach-integrator/integrator_ap.c
@@ -547,9 +547,7 @@ static void __init ap_init_of(void)
}
parent = soc_device_to_device(soc_dev);
-
- if (!IS_ERR_OR_NULL(parent))
- integrator_init_sysfs(parent, ap_sc_id);
+ integrator_init_sysfs(parent, ap_sc_id);
of_platform_populate(root, of_default_bus_match_table,
ap_auxdata_lookup, parent);
diff --git a/arch/arm/mach-integrator/integrator_cp.c
b/arch/arm/mach-integrator/integrator_cp.c
index 7322838..3fadadf 100644
--- a/arch/arm/mach-integrator/integrator_cp.c
+++ b/arch/arm/mach-integrator/integrator_cp.c
@@ -371,10 +371,7 @@ static void __init intcp_init_of(void)
}
parent = soc_device_to_device(soc_dev);
-
- if (!IS_ERR_OR_NULL(parent))
- integrator_init_sysfs(parent, intcp_sc_id);
-
+ integrator_init_sysfs(parent, intcp_sc_id);
of_platform_populate(root, of_default_bus_match_table,
intcp_auxdata_lookup, parent);
}
diff --git a/arch/arm/mach-omap2/omap_device.c
b/arch/arm/mach-omap2/omap_device.c
index e065daa..fabb32d 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -1157,20 +1157,17 @@ struct device *omap_device_get_by_hwmod_name(const char
*oh_name)
}
oh = omap_hwmod_lookup(oh_name);
- if (IS_ERR_OR_NULL(oh)) {
+ if (!oh) {
WARN(1, "%s: no hwmod for %s\n", __func__,
oh_name);
- return ERR_PTR(oh ? PTR_ERR(oh) : -ENODEV);
+ return -ENODEV;
}
- if (IS_ERR_OR_NULL(oh->od)) {
+ if (!oh->od) {
WARN(1, "%s: no omap_device for %s\n", __func__,
oh_name);
- return ERR_PTR(oh->od ? PTR_ERR(oh->od) : -ENODEV);
+ return -ENODEV;
}
- if (IS_ERR_OR_NULL(oh->od->pdev))
- return ERR_PTR(oh->od->pdev ? PTR_ERR(oh->od->pdev) : -ENODEV);
-
return &oh->od->pdev->dev;
}
EXPORT_SYMBOL(omap_device_get_by_hwmod_name);
diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c
index e2c291f..548547e 100644
--- a/arch/arm/mach-omap2/pm-debug.c
+++ b/arch/arm/mach-omap2/pm-debug.c
@@ -219,7 +219,7 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm,
void *dir)
return 0;
d = debugfs_create_dir(pwrdm->name, (struct dentry *)dir);
- if (!(IS_ERR_OR_NULL(d)))
+ if (d)
(void) debugfs_create_file("suspend", S_IRUGO|S_IWUSR, d,
(void *)pwrdm, &pwrdm_suspend_fops);
@@ -263,8 +263,8 @@ static int __init pm_dbg_init(void)
return 0;
d = debugfs_create_dir("pm_debug", NULL);
- if (IS_ERR_OR_NULL(d))
- return PTR_ERR(d);
+ if (!d)
+ return -EINVAL;
(void) debugfs_create_file("count", S_IRUGO,
d, (void *)DEBUG_FILE_COUNTERS, &debug_fops);
diff --git a/arch/arm/mach-omap2/powerdomain.c
b/arch/arm/mach-omap2/powerdomain.c
index dea62a9..36a6918 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -1054,7 +1054,7 @@ bool pwrdm_can_ever_lose_context(struct powerdomain
*pwrdm)
{
int i;
- if (IS_ERR_OR_NULL(pwrdm)) {
+ if (!pwrdm) {
pr_debug("powerdomain: %s: invalid powerdomain point
Re: [PATCH] Proposed removal of IS_ERR_OR_NULL() (was: Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface)
On Wed, 9 Jan 2013, Russell King - ARM Linux wrote:
> On Wed, Jan 09, 2013 at 10:27:53AM -0500, Nicolas Pitre wrote:
> > Anyone with good coccinelle skills around to deal with the users?
>
> I'm not sure that's a solution.
Well, I was thinking that coccinelle could handle the majority of the
354 users when the "fix" is obvious enough to be automated.
That said, if we want people to fix their code, it is probably necessary
to merge your patch right away so the warnings are actually being seen,
and revert it right before the final v3.8 release if the remaining
warnings are still too numerous. Repeat with next cycle.
> For example:
>
> err = gpio_request(en_vdd_1v05, "EN_VDD_1V05");
> if (err) {
> pr_err("%s: gpio_request failed: %d\n", __func__, err);
> return err;
> }
> ...
> regulator = regulator_get(NULL, "vdd_ldo0,vddio_pex_clk");
> if (IS_ERR_OR_NULL(regulator)) {
> pr_err("%s: regulator_get failed: %d\n", __func__,
>(int)PTR_ERR(regulator));
> goto err_reg;
> }
>
> regulator_enable(regulator);
>
> err = tegra_pcie_init(true, true);
> ...
> err_reg:
> gpio_free(en_vdd_1v05);
>
> return err;
> }
>
> Now, regulator_get() returns error-pointers for real errors when it's
> configured in. When regulator support is not configured, it returns
> NULL.
>
> So, one solution here would be:
>
> if (IS_ERR(regulator)) {
> err = PTR_ERR(regulator);
> pr_err("%s: regulator_get failed: %d\n", __func__, err);
> goto err_reg;
> }
>
> but leaves us with the question: is it safe to call tegra_pcie_init()
> without regulator support?
The best approach is to assume it is not, but unlike the current code,
this should be fixed by returning an appropriate error code instead of 0.
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Proposed removal of IS_ERR_OR_NULL() (was: Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface)
On Wed, Jan 09, 2013 at 10:27:53AM -0500, Nicolas Pitre wrote:
> Anyone with good coccinelle skills around to deal with the users?
I'm not sure that's a solution.
For example:
err = gpio_request(en_vdd_1v05, "EN_VDD_1V05");
if (err) {
pr_err("%s: gpio_request failed: %d\n", __func__, err);
return err;
}
...
regulator = regulator_get(NULL, "vdd_ldo0,vddio_pex_clk");
if (IS_ERR_OR_NULL(regulator)) {
pr_err("%s: regulator_get failed: %d\n", __func__,
(int)PTR_ERR(regulator));
goto err_reg;
}
regulator_enable(regulator);
err = tegra_pcie_init(true, true);
...
err_reg:
gpio_free(en_vdd_1v05);
return err;
}
Now, regulator_get() returns error-pointers for real errors when it's
configured in. When regulator support is not configured, it returns
NULL.
So, one solution here would be:
if (IS_ERR(regulator)) {
err = PTR_ERR(regulator);
pr_err("%s: regulator_get failed: %d\n", __func__, err);
goto err_reg;
}
but leaves us with the question: is it safe to call tegra_pcie_init()
without regulator support?
The problem there is that "fixing" this causes a behavioural change in
the code, and we don't know what effect that change may have.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Proposed removal of IS_ERR_OR_NULL() (was: Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface)
On Wed, Jan 9, 2013 at 3:04 PM, Russell King - ARM Linux wrote: > So, it seems there's some concensus building here, and it seems that > I've become the chosen victi^wvolunteer for this. So, here's a patch. > It's missing a Guns-supplied-by: tag though. > > From: Russell King > Subject: Mark IS_ERR_OR_NULL() deprecated > > IS_ERR_OR_NULL() attracts a lot of abuse: people use it without much > thought about it's effects. Common errors include: > 1. checking the returned pointer for functions defined as only > returning errno-pointer values, rather than using IS_ERR(). > This leads to: ptr = foo(); if (IS_ERR_OR_NULL(ptr)) return > PTR_ERR(ptr); > 2. using it to check functions which only ever return NULL on error, > thereby leading to another zero-error value return. > In the case of debugfs functions, these return errno-pointer values when > debugfs is configured out, which means code which blindly checks using > IS_ERR_OR_NULL() ends up returning errors, which is rather perverse for > something that's not implemented. > > Therefore, let's schedule it for removal in a few releases. > > Nicolas Pitre comments: >> I do agree with Russell here. Despite the original intentions behind >> IS_ERR_OR_NULL() which were certainly legitimate, the end result in >> practice is less reliable code with increased maintenance costs. >> Unlike other convenience macros in the kernel, this one is giving a >> false sense of correctness with too many people falling in the trap >> of using it just because it is available. >> >> I strongly think this macro should simply be removed from the source >> tree entirely and the code reverted to explicit tests against NULL >> when appropriate. > > Suggested-by: David Howells > Tape-measuring-service-offered-by: Will Deacon > Victim-for-firing-sqad: Russell King > Signed-off-by: Russell King Acked-by: Grant Likely I fully agree with doing this. While I'm not a fan of the PTR_ERR pattern, this does (hopefully) make users think just a little bit more about it. > --- > Ok, so I'm in the firing line for suggesting this, but it appears > several people wish this to happen. > > I'm not intending to push this patch forwards _just_ yet: we need to > sort out the existing users _first_ to prevent the kernel turning into > one hell of a mess of warnings. I currently see 355 users. That's a lot, but not inconceivable for an auditing effort for pulling them out instead of scheduling for future removal. g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Proposed removal of IS_ERR_OR_NULL() (was: Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface)
On Wed, 9 Jan 2013, Russell King - ARM Linux wrote:
> So, it seems there's some concensus building here, and it seems that
> I've become the chosen victi^wvolunteer for this. So, here's a patch.
> It's missing a Guns-supplied-by: tag though.
Guns-supplied-by: NRA (obviously)
> From: Russell King
> Subject: Mark IS_ERR_OR_NULL() deprecated
>
> IS_ERR_OR_NULL() attracts a lot of abuse: people use it without much
> thought about it's effects. Common errors include:
> 1. checking the returned pointer for functions defined as only
> returning errno-pointer values, rather than using IS_ERR().
> This leads to: ptr = foo(); if (IS_ERR_OR_NULL(ptr)) return
> PTR_ERR(ptr);
> 2. using it to check functions which only ever return NULL on error,
> thereby leading to another zero-error value return.
> In the case of debugfs functions, these return errno-pointer values when
> debugfs is configured out, which means code which blindly checks using
> IS_ERR_OR_NULL() ends up returning errors, which is rather perverse for
> something that's not implemented.
>
> Therefore, let's schedule it for removal in a few releases.
>
> Nicolas Pitre comments:
> > I do agree with Russell here. Despite the original intentions behind
> > IS_ERR_OR_NULL() which were certainly legitimate, the end result in
> > practice is less reliable code with increased maintenance costs.
> > Unlike other convenience macros in the kernel, this one is giving a
> > false sense of correctness with too many people falling in the trap
> > of using it just because it is available.
> >
> > I strongly think this macro should simply be removed from the source
> > tree entirely and the code reverted to explicit tests against NULL
> > when appropriate.
>
> Suggested-by: David Howells
> Tape-measuring-service-offered-by: Will Deacon
> Victim-for-firing-sqad: Russell King
> Signed-off-by: Russell King
Acked-by: Nicolas Pitre
Anyone with good coccinelle skills around to deal with the users?
> ---
> Ok, so I'm in the firing line for suggesting this, but it appears
> several people wish this to happen.
>
> I'm not intending to push this patch forwards _just_ yet: we need to
> sort out the existing users _first_ to prevent the kernel turning into
> one hell of a mess of warnings.
>
> include/linux/err.h | 17 -
> 1 files changed, 16 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/err.h b/include/linux/err.h
> index f2edce2..d5a85df 100644
> --- a/include/linux/err.h
> +++ b/include/linux/err.h
> @@ -34,7 +34,22 @@ static inline long __must_check IS_ERR(const void *ptr)
> return IS_ERR_VALUE((unsigned long)ptr);
> }
>
> -static inline long __must_check IS_ERR_OR_NULL(const void *ptr)
> +/*
> + * IS_ERR_OR_NULL() attracts a lot of abuse: people use it without much
> + * thought about it's effects. Common errors include:
> + * 1. checking the returned pointer for functions defined as only returning
> + * errno-pointer values, rather than using IS_ERR().
> + * This leads to: ptr = foo(); if (IS_ERR_OR_NULL(ptr)) return
> PTR_ERR(ptr);
> + * 2. using it to check functions which only ever return NULL on error,
> + * thereby leading to another zero-error value return.
> + * In the case of debugfs functions, these return errno-pointer values when
> + * debugfs is configured out, which means code which blindly checks using
> + * IS_ERR_OR_NULL() ends up returning errors, which is rather perverse for
> + * something that's not implemented.
> + *
> + * Therefore, let's schedule it for removal in a few releases.
> + */
> +static inline long __must_check __deprecated IS_ERR_OR_NULL(const void *ptr)
> {
> return !ptr || IS_ERR_VALUE((unsigned long)ptr);
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Proposed removal of IS_ERR_OR_NULL() (was: Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface)
On Wednesday 09 January 2013, Grant Likely wrote: > > Suggested-by: David Howells > > Tape-measuring-service-offered-by: Will Deacon > > Victim-for-firing-sqad: Russell King > > Signed-off-by: Russell King > > Acked-by: Grant Likely Acked-by: Arnd Bergmann -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Proposed removal of IS_ERR_OR_NULL() (was: Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface)
So, it seems there's some concensus building here, and it seems that
I've become the chosen victi^wvolunteer for this. So, here's a patch.
It's missing a Guns-supplied-by: tag though.
From: Russell King
Subject: Mark IS_ERR_OR_NULL() deprecated
IS_ERR_OR_NULL() attracts a lot of abuse: people use it without much
thought about it's effects. Common errors include:
1. checking the returned pointer for functions defined as only
returning errno-pointer values, rather than using IS_ERR().
This leads to: ptr = foo(); if (IS_ERR_OR_NULL(ptr)) return
PTR_ERR(ptr);
2. using it to check functions which only ever return NULL on error,
thereby leading to another zero-error value return.
In the case of debugfs functions, these return errno-pointer values when
debugfs is configured out, which means code which blindly checks using
IS_ERR_OR_NULL() ends up returning errors, which is rather perverse for
something that's not implemented.
Therefore, let's schedule it for removal in a few releases.
Nicolas Pitre comments:
> I do agree with Russell here. Despite the original intentions behind
> IS_ERR_OR_NULL() which were certainly legitimate, the end result in
> practice is less reliable code with increased maintenance costs.
> Unlike other convenience macros in the kernel, this one is giving a
> false sense of correctness with too many people falling in the trap
> of using it just because it is available.
>
> I strongly think this macro should simply be removed from the source
> tree entirely and the code reverted to explicit tests against NULL
> when appropriate.
Suggested-by: David Howells
Tape-measuring-service-offered-by: Will Deacon
Victim-for-firing-sqad: Russell King
Signed-off-by: Russell King
---
Ok, so I'm in the firing line for suggesting this, but it appears
several people wish this to happen.
I'm not intending to push this patch forwards _just_ yet: we need to
sort out the existing users _first_ to prevent the kernel turning into
one hell of a mess of warnings.
include/linux/err.h | 17 -
1 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/include/linux/err.h b/include/linux/err.h
index f2edce2..d5a85df 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -34,7 +34,22 @@ static inline long __must_check IS_ERR(const void *ptr)
return IS_ERR_VALUE((unsigned long)ptr);
}
-static inline long __must_check IS_ERR_OR_NULL(const void *ptr)
+/*
+ * IS_ERR_OR_NULL() attracts a lot of abuse: people use it without much
+ * thought about it's effects. Common errors include:
+ * 1. checking the returned pointer for functions defined as only returning
+ * errno-pointer values, rather than using IS_ERR().
+ * This leads to: ptr = foo(); if (IS_ERR_OR_NULL(ptr)) return
PTR_ERR(ptr);
+ * 2. using it to check functions which only ever return NULL on error,
+ * thereby leading to another zero-error value return.
+ * In the case of debugfs functions, these return errno-pointer values when
+ * debugfs is configured out, which means code which blindly checks using
+ * IS_ERR_OR_NULL() ends up returning errors, which is rather perverse for
+ * something that's not implemented.
+ *
+ * Therefore, let's schedule it for removal in a few releases.
+ */
+static inline long __must_check __deprecated IS_ERR_OR_NULL(const void *ptr)
{
return !ptr || IS_ERR_VALUE((unsigned long)ptr);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface
On Wed, 9 Jan 2013, Russell King - ARM Linux wrote: > On Wed, Jan 09, 2013 at 10:44:14AM +, Russell King - ARM Linux wrote: > > On Wed, Jan 09, 2013 at 10:35:22AM +, Arnd Bergmann wrote: > > > On Wednesday 09 January 2013, Alexandre Courbot wrote: > > > > On Tue, Jan 8, 2013 at 9:59 PM, Arnd Bergmann wrote: > > > > > Please avoid the use of IS_ERR_OR_NULL(), especially on interfaces you > > > > > introduce yourself. AFAICT, gpiod_get cannot return NULL, so you > > > > > should not check for that. > > > > > > > > Sure - you sound like IS_ERR_OR_NULL() is generally considered evil, > > > > > > Correct. > > > > > > > may I ask why this is the case? > > > > > > It's very hard to get right: either you are interested in the error code, > > > and then you don't have one in some cases, or you don't care but have > > > to check for it anyway. When you define a function, just make it clear > > > what the expected return values are, either NULL for error or a negative > > > ERR_PTR value, but not both. > > > > Indeed, and any code which does this: > > > > if (IS_ERR_OR_NULL(ptr)) > > return PTR_ERR(ptr); > > > > is buggy because on NULL it returns 0, which is generally accepted as being > > "success". > [ examples of broken code skipped ] > These are just a few of the issues I've picked out at random from grepping > the kernel source for IS_ERR_OR_NULL(). Yes, there's some valid use cases > but the above are all horrid, buggy or down right wrong, and I wouldn't be > at all surprised if that was all too common. I do agree with Russell here. Despite the original intentions behind IS_ERR_OR_NULL() which were certainly legitimate, the end result in practice is less reliable code with increased maintenance costs. Unlike other convenience macros in the kernel, this one is giving a false sense of correctness with too many people falling in the trap of using it just because it is available. I strongly think this macro should simply be removed from the source tree entirely and the code reverted to explicit tests against NULL when appropriate. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface
On Wednesday 09 January 2013, Russell King - ARM Linux wrote:
> d = debugfs_create_dir("pm_debug", NULL);
> if (IS_ERR_OR_NULL(d))
> return PTR_ERR(d);
>
> Well, covered above. NULL is success here.
This one is actually worse, because in case of debugfs_create_dir,
a negative error code is documented to mean "success": The debugfs
functions intentionally return ERR_PTR(-ENODEV) when debugfs is
disabled so that any code checking for NULL pretends it is a valid
pointer, but that code is only allowed to pass this pointer into
other debugfs functions that are also stubbed out and never
dereference it.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface
On Wed, Jan 09, 2013 at 10:44:14AM +, Russell King - ARM Linux wrote:
> On Wed, Jan 09, 2013 at 10:35:22AM +, Arnd Bergmann wrote:
> > On Wednesday 09 January 2013, Alexandre Courbot wrote:
> > > On Tue, Jan 8, 2013 at 9:59 PM, Arnd Bergmann wrote:
> > > > Please avoid the use of IS_ERR_OR_NULL(), especially on interfaces you
> > > > introduce yourself. AFAICT, gpiod_get cannot return NULL, so you
> > > > should not check for that.
> > >
> > > Sure - you sound like IS_ERR_OR_NULL() is generally considered evil,
> >
> > Correct.
> >
> > > may I ask why this is the case?
> >
> > It's very hard to get right: either you are interested in the error code,
> > and then you don't have one in some cases, or you don't care but have
> > to check for it anyway. When you define a function, just make it clear
> > what the expected return values are, either NULL for error or a negative
> > ERR_PTR value, but not both.
>
> Indeed, and any code which does this:
>
> if (IS_ERR_OR_NULL(ptr))
> return PTR_ERR(ptr);
>
> is buggy because on NULL it returns 0, which is generally accepted as being
> "success".
oh = omap_hwmod_lookup(oh_name);
if (!oh) {
oh = omap_hwmod_lookup(oh_name);
if (IS_ERR_OR_NULL(oh)) {
Does this function return NULL on errors or an errno-encoded-pointer on
errors?
d = debugfs_create_dir("pm_debug", NULL);
if (IS_ERR_OR_NULL(d))
return PTR_ERR(d);
Well, covered above. NULL is success here.
err = gpio_request(en_vdd_1v05, "EN_VDD_1V05");
if (err) {
pr_err("%s: gpio_request failed: %d\n", __func__, err);
return err;
}
gpio_direction_output(en_vdd_1v05, 1);
regulator = regulator_get(NULL, "vdd_ldo0,vddio_pex_clk");
if (IS_ERR_OR_NULL(regulator)) {
pr_err("%s: regulator_get failed: %d\n", __func__,
(int)PTR_ERR(regulator));
goto err_reg;
}
...
err_reg:
gpio_free(en_vdd_1v05);
return err;
}
So 'err' here is never set. when IS_ERR_OR_NULL evaluates true.
Setting 'err' to PTR_ERR(regulator) is not correct because a NULL return
sets 'err' to zero. Same here:
/* create property id */
ret = ipp_create_id(&ctx->prop_idr, &ctx->prop_lock, c_node,
&property->prop_id);
if (ret) {
DRM_ERROR("failed to create id.\n");
goto err_clear;
}
...
c_node->start_work = ipp_create_cmd_work();
if (IS_ERR_OR_NULL(c_node->start_work)) {
DRM_ERROR("failed to create start work.\n");
goto err_clear;
}
c_node->stop_work = ipp_create_cmd_work();
if (IS_ERR_OR_NULL(c_node->stop_work)) {
DRM_ERROR("failed to create stop work.\n");
goto err_free_start;
}
c_node->event_work = ipp_create_event_work();
if (IS_ERR_OR_NULL(c_node->event_work)) {
DRM_ERROR("failed to create event work.\n");
goto err_free_stop;
}
We also have it cropping up as a way to 'verify' function arguments
are correct:
int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
struct gpd_timing_data *td)
{
if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
return -EINVAL;
And then we have this beauty in USB code:
if (!IS_ERR_OR_NULL(udc->transceiver))
(void) otg_set_peripheral(udc->transceiver->otg, NULL);
else
pullup_disable(udc);
...
seq_printf(s,
"UDC rev %d.%d, fifo mode %d, gadget %s\n"
"hmc %d, transceiver %s\n",
tmp >> 4, tmp & 0xf,
fifo_mode,
udc->driver ? udc->driver->driver.name : "(none)",
HMC,
udc->transceiver
? udc->transceiver->label
: (cpu_is_omap1710()
? "external" : "(none)"));
If udc->transceiver were to be an error code, the above will segfault or
access memory at the top of memory space.
These are just a few of the issues I've picked out at random from grepping
the kernel source for IS_ERR_OR_NULL(). Yes, there's some valid use cases
but the above are all horrid, buggy or down right wrong, and I wouldn't be
at all surprised if that was all too common.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface
On Wed, Jan 09, 2013 at 10:35:22AM +, Arnd Bergmann wrote: > On Wednesday 09 January 2013, Alexandre Courbot wrote: > > On Tue, Jan 8, 2013 at 9:59 PM, Arnd Bergmann wrote: > > > Please avoid the use of IS_ERR_OR_NULL(), especially on interfaces you > > > introduce yourself. AFAICT, gpiod_get cannot return NULL, so you > > > should not check for that. > > > > Sure - you sound like IS_ERR_OR_NULL() is generally considered evil, > > Correct. > > > may I ask why this is the case? > > It's very hard to get right: either you are interested in the error code, > and then you don't have one in some cases, or you don't care but have > to check for it anyway. When you define a function, just make it clear > what the expected return values are, either NULL for error or a negative > ERR_PTR value, but not both. Indeed, and any code which does this: if (IS_ERR_OR_NULL(ptr)) return PTR_ERR(ptr); is buggy because on NULL it returns 0, which is generally accepted as being "success". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface
On Wednesday 09 January 2013, Alexandre Courbot wrote: > On Tue, Jan 8, 2013 at 9:59 PM, Arnd Bergmann wrote: > > Please avoid the use of IS_ERR_OR_NULL(), especially on interfaces you > > introduce yourself. AFAICT, gpiod_get cannot return NULL, so you > > should not check for that. > > Sure - you sound like IS_ERR_OR_NULL() is generally considered evil, Correct. > may I ask why this is the case? It's very hard to get right: either you are interested in the error code, and then you don't have one in some cases, or you don't care but have to check for it anyway. When you define a function, just make it clear what the expected return values are, either NULL for error or a negative ERR_PTR value, but not both. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface
On Wed, Jan 09, 2013 at 10:06:16AM +0900, Alexandre Courbot wrote: > On Tue, Jan 8, 2013 at 9:59 PM, Arnd Bergmann wrote: > > Please avoid the use of IS_ERR_OR_NULL(), especially on interfaces you > > introduce yourself. AFAICT, gpiod_get cannot return NULL, so you > > should not check for that. > > Sure - you sound like IS_ERR_OR_NULL() is generally considered evil, > may I ask why this is the case? I think I've explained that in the past; many people just do not think. They just use whatever macro they feel like is the right one. We keep seeing this, and this is a persistent problem. It's getting to be more of a problem because people are starting to argue back when you point out that they're wrong. People are even starting to believe that documentation which specifies explicitly "values where IS_ERR() is true are considered errors, everything else is valid" means that the use of IS_ERR_OR_NULL() in such cases is permissible. (I've had such an argument with two people recently.) So, interfaces which have well defined return values and even interfaces which specify _how_ errors should be checked end up being checked with the wrong macros. People constantly translate IS_ERR() to IS_ERR_OR_NULL() even when it's inappropriate. People don't think and people don't read documentation. People don't remember this level of detail. Whatever the excuse, the problem remains. IS_ERR_OR_NULL() always gets used inappropriately and without any regard to whether it's correct or not. So yes, IS_ERR_OR_NULL() _is_ pure evil. IMHO this macro is doing more harm than good. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface
On Tue, Jan 8, 2013 at 9:59 PM, Arnd Bergmann wrote: > Please avoid the use of IS_ERR_OR_NULL(), especially on interfaces you > introduce yourself. AFAICT, gpiod_get cannot return NULL, so you > should not check for that. Sure - you sound like IS_ERR_OR_NULL() is generally considered evil, may I ask why this is the case? >> + *dr = desc; >> + devres_add(dev, dr); >> + >> + return 0; >> +} > > I'm pretty sure you meant to write 'return desc;' here. Absolutely, thanks. Alex. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface
On Tuesday 08 January 2013, Alexandre Courbot wrote:
> +struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
> +const char *con_id)
> +{
> + struct gpio_desc **dr;
> + struct gpio_desc *desc;
> +
> + dr = devres_alloc(devm_gpio_release, sizeof(struct gpio_desc *),
> + GFP_KERNEL);
> + if (!dr)
> + return ERR_PTR(-ENOMEM);
> +
> + desc = gpiod_get(dev, con_id);
> + if (IS_ERR_OR_NULL(desc)) {
> + devres_free(dr);
> + return desc;
> + }
Please avoid the use of IS_ERR_OR_NULL(), especially on interfaces you
introduce yourself. AFAICT, gpiod_get cannot return NULL, so you
should not check for that.
> + *dr = desc;
> + devres_add(dev, dr);
> +
> + return 0;
> +}
I'm pretty sure you meant to write 'return desc;' here.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface
With the current API, GPIOs are represented by a unique integer. This
causes problems in terms of security (GPIO numbers can be arbitrarily
forged and used without proper allocation) and flexibility (the maximum
number of GPIOs that the system can handle is fixed at compilation time
and a static array of that size is allocated to store the GPIO
descriptors).
This patch introduces an alternative GPIO API that uses opaque handlers
and refactors GPIOlib's internals to work with these handlers instead of
GPIO numbers. The former integer-based API is still available as a light
wrapper around this new API.
Signed-off-by: Alexandre Courbot
---
drivers/gpio/devres.c | 59 +++--
drivers/gpio/gpiolib.c| 486 +++---
include/asm-generic/gpio.h| 176 ++-
include/linux/gpio/consumer.h | 64 ++
include/linux/gpio/driver.h | 112 ++
5 files changed, 552 insertions(+), 345 deletions(-)
create mode 100644 include/linux/gpio/consumer.h
create mode 100644 include/linux/gpio/driver.h
diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c
index 1077754..b7a9ad6 100644
--- a/drivers/gpio/devres.c
+++ b/drivers/gpio/devres.c
@@ -15,22 +15,24 @@
*/
#include
+#include
#include
+#include
#include
#include
static void devm_gpio_release(struct device *dev, void *res)
{
- unsigned *gpio = res;
+ struct gpio_desc **desc = res;
- gpio_free(*gpio);
+ gpiod_put(*desc);
}
static int devm_gpio_match(struct device *dev, void *res, void *data)
{
- unsigned *this = res, *gpio = data;
+ struct gpio_desc **this = res, **desc = data;
- return *this == *gpio;
+ return *this == *desc;
}
/**
@@ -50,10 +52,11 @@ static int devm_gpio_match(struct device *dev, void *res,
void *data)
int devm_gpio_request(struct device *dev, unsigned gpio, const char *label)
{
- unsigned *dr;
+ struct gpio_desc **dr;
int rc;
- dr = devres_alloc(devm_gpio_release, sizeof(unsigned), GFP_KERNEL);
+ dr = devres_alloc(devm_gpio_release, sizeof(struct gpio_desc *),
+ GFP_KERNEL);
if (!dr)
return -ENOMEM;
@@ -63,7 +66,7 @@ int devm_gpio_request(struct device *dev, unsigned gpio,
const char *label)
return rc;
}
- *dr = gpio;
+ *dr = gpio_to_desc(gpio);
devres_add(dev, dr);
return 0;
@@ -80,10 +83,11 @@ EXPORT_SYMBOL(devm_gpio_request);
int devm_gpio_request_one(struct device *dev, unsigned gpio,
unsigned long flags, const char *label)
{
- unsigned *dr;
+ struct gpio_desc **dr;
int rc;
- dr = devres_alloc(devm_gpio_release, sizeof(unsigned), GFP_KERNEL);
+ dr = devres_alloc(devm_gpio_release, sizeof(struct gpio_desc *),
+ GFP_KERNEL);
if (!dr)
return -ENOMEM;
@@ -93,7 +97,7 @@ int devm_gpio_request_one(struct device *dev, unsigned gpio,
return rc;
}
- *dr = gpio;
+ *dr = gpio_to_desc(gpio);
devres_add(dev, dr);
return 0;
@@ -112,8 +116,39 @@ EXPORT_SYMBOL(devm_gpio_request_one);
*/
void devm_gpio_free(struct device *dev, unsigned int gpio)
{
+ struct gpio_desc *desc = gpio_to_desc(gpio);
- WARN_ON(devres_release(dev, devm_gpio_release, devm_gpio_match,
- &gpio));
+ devm_gpiod_put(dev, desc);
}
EXPORT_SYMBOL(devm_gpio_free);
+
+struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
+const char *con_id)
+{
+ struct gpio_desc **dr;
+ struct gpio_desc *desc;
+
+ dr = devres_alloc(devm_gpio_release, sizeof(struct gpio_desc *),
+ GFP_KERNEL);
+ if (!dr)
+ return ERR_PTR(-ENOMEM);
+
+ desc = gpiod_get(dev, con_id);
+ if (IS_ERR_OR_NULL(desc)) {
+ devres_free(dr);
+ return desc;
+ }
+
+ *dr = desc;
+ devres_add(dev, dr);
+
+ return 0;
+}
+EXPORT_SYMBOL(devm_gpiod_get);
+
+void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)
+{
+ WARN_ON(devres_release(dev, devm_gpio_release, devm_gpio_match,
+ &desc));
+}
+EXPORT_SYMBOL(devm_gpiod_put);
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 199fca1..d04b90b 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -11,6 +11,8 @@
#include
#include
#include
+#include
+#include
#define CREATE_TRACE_POINTS
#include
@@ -76,6 +78,9 @@ static struct gpio_desc gpio_desc[ARCH_NR_GPIOS];
static DEFINE_IDR(dirent_idr);
#endif
+static int gpiod_request(struct gpio_desc *desc, const char *label);
+static void gpiod_free(struct gpio_desc *desc);
+
static inline void desc_set_label(struct gpio_desc *d, const char *label)
{
#ifdef CONFIG_DEBUG_FS
@@ -83,6 +88,38 @@ static inline
