From: lee.jones@linaro.org (Lee Jones)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 4/4] pinctrl/abx500: destroy mutex if returning early due to error
Date: Thu, 31 Jan 2013 08:01:51 +0000 [thread overview]
Message-ID: <20130131080151.GA5301@gmail.com> (raw)
In-Reply-To: <51097E35.6080100@wwwdotorg.org>
On Wed, 30 Jan 2013, Stephen Warren wrote:
> On 01/30/2013 12:40 PM, Linus Walleij wrote:
> > From: Lee Jones <lee.jones@linaro.org>
> >
> > Current failure path neglects to mutex_destroy() before returning
> > an error due to an invalid parameter or an error received from
> > gpiochip_add(). This patch aims to remedy that behaviour.
>
> > diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
>
> > @@ -1155,11 +1155,13 @@ static int abx500_gpio_probe(struct platform_device *pdev)
> > default:
> > dev_err(&pdev->dev, "Unsupported pinctrl sub driver (%d)\n",
> > (int) platid->driver_data);
> > + mutex_destroy(&pct->lock);
> > return -EINVAL;
>
> Especially given there's already a label out_free which performs this
> mutex_destroy(), those last two lines would be better as:
>
> ret = -EINVAL;
> goto out_free;
Yes, that's one way of doing it. I figured it was 6 of one and half a
dozen of the other to be honest.
Either I:
+ mutex_destroy(&pct->lock);
Or:
+ ret = -EINVAL;
+ goto out_free;
- return -EINVAL;
I figured the smallest diff would be best. To be honest, I'm not
bothered either way. If it offends you, I can do it the other way, no
problem. Just let me know quick, so I can get the fixed up patch to
Linus.
NB: There is no 'out_free:' at this point, it has already been
removed.
I'll leave it up to you.
> > }
> >
> > if (!pct->soc) {
> > dev_err(&pdev->dev, "Invalid SOC data\n");
> > + mutex_destroy(&pct->lock);
> > return -EINVAL;
>
> Same there.
>
> > }
> >
> > @@ -1176,6 +1178,7 @@ static int abx500_gpio_probe(struct platform_device *pdev)
> > ret = gpiochip_add(&pct->chip);
> > if (ret) {
> > dev_err(&pdev->dev, "unable to add gpiochip: %d\n", ret);
> > + mutex_destroy(&pct->lock);
> > goto out_rem_irq;
>
> And here, just change the goto target to out_free rather than adding the
> mutex_destroy() call.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee.jones@linaro.org>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: Linus Walleij <linus.walleij@stericsson.com>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
Stephen Warren <swarren@nvidia.com>,
Anmar Oueja <anmar.oueja@linaro.org>,
Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [PATCH 4/4] pinctrl/abx500: destroy mutex if returning early due to error
Date: Thu, 31 Jan 2013 08:01:51 +0000 [thread overview]
Message-ID: <20130131080151.GA5301@gmail.com> (raw)
In-Reply-To: <51097E35.6080100@wwwdotorg.org>
On Wed, 30 Jan 2013, Stephen Warren wrote:
> On 01/30/2013 12:40 PM, Linus Walleij wrote:
> > From: Lee Jones <lee.jones@linaro.org>
> >
> > Current failure path neglects to mutex_destroy() before returning
> > an error due to an invalid parameter or an error received from
> > gpiochip_add(). This patch aims to remedy that behaviour.
>
> > diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
>
> > @@ -1155,11 +1155,13 @@ static int abx500_gpio_probe(struct platform_device *pdev)
> > default:
> > dev_err(&pdev->dev, "Unsupported pinctrl sub driver (%d)\n",
> > (int) platid->driver_data);
> > + mutex_destroy(&pct->lock);
> > return -EINVAL;
>
> Especially given there's already a label out_free which performs this
> mutex_destroy(), those last two lines would be better as:
>
> ret = -EINVAL;
> goto out_free;
Yes, that's one way of doing it. I figured it was 6 of one and half a
dozen of the other to be honest.
Either I:
+ mutex_destroy(&pct->lock);
Or:
+ ret = -EINVAL;
+ goto out_free;
- return -EINVAL;
I figured the smallest diff would be best. To be honest, I'm not
bothered either way. If it offends you, I can do it the other way, no
problem. Just let me know quick, so I can get the fixed up patch to
Linus.
NB: There is no 'out_free:' at this point, it has already been
removed.
I'll leave it up to you.
> > }
> >
> > if (!pct->soc) {
> > dev_err(&pdev->dev, "Invalid SOC data\n");
> > + mutex_destroy(&pct->lock);
> > return -EINVAL;
>
> Same there.
>
> > }
> >
> > @@ -1176,6 +1178,7 @@ static int abx500_gpio_probe(struct platform_device *pdev)
> > ret = gpiochip_add(&pct->chip);
> > if (ret) {
> > dev_err(&pdev->dev, "unable to add gpiochip: %d\n", ret);
> > + mutex_destroy(&pct->lock);
> > goto out_rem_irq;
>
> And here, just change the goto target to out_free rather than adding the
> mutex_destroy() call.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
next prev parent reply other threads:[~2013-01-31 8:01 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-30 19:40 [PATCH 4/4] pinctrl/abx500: destroy mutex if returning early due to error Linus Walleij
2013-01-30 19:40 ` Linus Walleij
2013-01-30 20:10 ` Stephen Warren
2013-01-30 20:10 ` Stephen Warren
2013-01-31 8:01 ` Lee Jones [this message]
2013-01-31 8:01 ` Lee Jones
2013-01-31 17:28 ` Stephen Warren
2013-01-31 17:28 ` Stephen Warren
2013-01-31 17:45 ` Lee Jones
2013-01-31 17:45 ` Lee Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130131080151.GA5301@gmail.com \
--to=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.