* [PATCH -next] power: supply: ltc3651-charger: Fix wrong pointer passed to PTR_ERR()
@ 2017-05-17 12:11 Wei Yongjun
2017-05-19 0:59 ` Dmitry Torokhov
2017-06-07 20:16 ` Sebastian Reichel
0 siblings, 2 replies; 5+ messages in thread
From: Wei Yongjun @ 2017-05-17 12:11 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Torokhov; +Cc: Wei Yongjun, linux-pm
From: Wei Yongjun <weiyongjun1@huawei.com>
PTR_ERR should access the value just tested by IS_ERR, otherwise
the wrong error code will be returned.
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/power/supply/ltc3651-charger.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/power/supply/ltc3651-charger.c b/drivers/power/supply/ltc3651-charger.c
index 5f8d5c0..eea63ff 100644
--- a/drivers/power/supply/ltc3651-charger.c
+++ b/drivers/power/supply/ltc3651-charger.c
@@ -110,21 +110,21 @@ static int ltc3651_charger_probe(struct platform_device *pdev)
ltc3651_charger->acpr_gpio = devm_gpiod_get(&pdev->dev,
"lltc,acpr", GPIOD_IN);
if (IS_ERR(ltc3651_charger->acpr_gpio)) {
- ret = PTR_ERR(ltc3651_charger->charger);
+ ret = PTR_ERR(ltc3651_charger->acpr_gpio);
dev_err(&pdev->dev, "Failed to acquire acpr GPIO: %d\n", ret);
return ret;
}
ltc3651_charger->fault_gpio = devm_gpiod_get_optional(&pdev->dev,
"lltc,fault", GPIOD_IN);
if (IS_ERR(ltc3651_charger->fault_gpio)) {
- ret = PTR_ERR(ltc3651_charger->charger);
+ ret = PTR_ERR(ltc3651_charger->fault_gpio);
dev_err(&pdev->dev, "Failed to acquire fault GPIO: %d\n", ret);
return ret;
}
ltc3651_charger->chrg_gpio = devm_gpiod_get_optional(&pdev->dev,
"lltc,chrg", GPIOD_IN);
if (IS_ERR(ltc3651_charger->chrg_gpio)) {
- ret = PTR_ERR(ltc3651_charger->charger);
+ ret = PTR_ERR(ltc3651_charger->chrg_gpio);
dev_err(&pdev->dev, "Failed to acquire chrg GPIO: %d\n", ret);
return ret;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH -next] power: supply: ltc3651-charger: Fix wrong pointer passed to PTR_ERR()
2017-05-17 12:11 [PATCH -next] power: supply: ltc3651-charger: Fix wrong pointer passed to PTR_ERR() Wei Yongjun
@ 2017-05-19 0:59 ` Dmitry Torokhov
2017-05-19 1:27 ` weiyongjun (A)
2017-06-07 20:16 ` Sebastian Reichel
1 sibling, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2017-05-19 0:59 UTC (permalink / raw)
To: Wei Yongjun; +Cc: Sebastian Reichel, Wei Yongjun, linux-pm
On Wed, May 17, 2017 at 12:11:32PM +0000, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> PTR_ERR should access the value just tested by IS_ERR, otherwise
> the wrong error code will be returned.
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
This does fix the issue, but maybe we want to use PTR_ERR_OR_ZERO()?
ltc3651_charger->acpr_gpio = devm_gpiod_get(&pdev->dev,
"lltc,acpr", GPIOD_IN);
ret = PTR_ERR_OR_ZERO(ltc3651_charger->acpr_gpio);
if (ret)
...
Or even
err = PTR_ERR_OR_ZERO((ltc3651_charger->acpr_gpio =
devm_gpiod_get(&pdev->dev, "lltc,acpr", GPIOD_IN)));
if (err)
...
To make sure we never mix up pointer. Maybe we need ERR_OR_ASSIGN(ptr1,
ptr2) to avoid the ugly assignment in argument above...
Thanks.
> ---
> drivers/power/supply/ltc3651-charger.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/power/supply/ltc3651-charger.c b/drivers/power/supply/ltc3651-charger.c
> index 5f8d5c0..eea63ff 100644
> --- a/drivers/power/supply/ltc3651-charger.c
> +++ b/drivers/power/supply/ltc3651-charger.c
> @@ -110,21 +110,21 @@ static int ltc3651_charger_probe(struct platform_device *pdev)
> ltc3651_charger->acpr_gpio = devm_gpiod_get(&pdev->dev,
> "lltc,acpr", GPIOD_IN);
> if (IS_ERR(ltc3651_charger->acpr_gpio)) {
> - ret = PTR_ERR(ltc3651_charger->charger);
> + ret = PTR_ERR(ltc3651_charger->acpr_gpio);
> dev_err(&pdev->dev, "Failed to acquire acpr GPIO: %d\n", ret);
> return ret;
> }
> ltc3651_charger->fault_gpio = devm_gpiod_get_optional(&pdev->dev,
> "lltc,fault", GPIOD_IN);
> if (IS_ERR(ltc3651_charger->fault_gpio)) {
> - ret = PTR_ERR(ltc3651_charger->charger);
> + ret = PTR_ERR(ltc3651_charger->fault_gpio);
> dev_err(&pdev->dev, "Failed to acquire fault GPIO: %d\n", ret);
> return ret;
> }
> ltc3651_charger->chrg_gpio = devm_gpiod_get_optional(&pdev->dev,
> "lltc,chrg", GPIOD_IN);
> if (IS_ERR(ltc3651_charger->chrg_gpio)) {
> - ret = PTR_ERR(ltc3651_charger->charger);
> + ret = PTR_ERR(ltc3651_charger->chrg_gpio);
> dev_err(&pdev->dev, "Failed to acquire chrg GPIO: %d\n", ret);
> return ret;
> }
>
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH -next] power: supply: ltc3651-charger: Fix wrong pointer passed to PTR_ERR()
2017-05-19 0:59 ` Dmitry Torokhov
@ 2017-05-19 1:27 ` weiyongjun (A)
2017-05-19 22:07 ` Dmitry Torokhov
0 siblings, 1 reply; 5+ messages in thread
From: weiyongjun (A) @ 2017-05-19 1:27 UTC (permalink / raw)
To: Dmitry Torokhov, Wei Yongjun; +Cc: Sebastian Reichel, linux-pm@vger.kernel.org
Hi, Dmitry
> Subject: Re: [PATCH -next] power: supply: ltc3651-charger: Fix wrong pointer
> passed to PTR_ERR()
>
> On Wed, May 17, 2017 at 12:11:32PM +0000, Wei Yongjun wrote:
> > From: Wei Yongjun <weiyongjun1@huawei.com>
> >
> > PTR_ERR should access the value just tested by IS_ERR, otherwise
> > the wrong error code will be returned.
> >
> > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
>
> This does fix the issue, but maybe we want to use PTR_ERR_OR_ZERO()?
>
> ltc3651_charger->acpr_gpio = devm_gpiod_get(&pdev->dev,
> "lltc,acpr", GPIOD_IN);
> ret = PTR_ERR_OR_ZERO(ltc3651_charger->acpr_gpio);
> if (ret)
> ...
>
> Or even
>
> err = PTR_ERR_OR_ZERO((ltc3651_charger->acpr_gpio =
> devm_gpiod_get(&pdev->dev, "lltc,acpr", GPIOD_IN)));
> if (err)
> ...
>
> To make sure we never mix up pointer. Maybe we need
> ERR_OR_ASSIGN(ptr1,
> ptr2) to avoid the ugly assignment in argument above...
>
I have checked devm_gpiod_get() and seems it never return NULL. And the other
places call devm_gpiod_get() never check for NULL return. So I think PTR_ERR_OR_ZERO()
is not need here.
Regards,
Yongjun Wei
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH -next] power: supply: ltc3651-charger: Fix wrong pointer passed to PTR_ERR()
2017-05-19 1:27 ` weiyongjun (A)
@ 2017-05-19 22:07 ` Dmitry Torokhov
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2017-05-19 22:07 UTC (permalink / raw)
To: weiyongjun (A); +Cc: Wei Yongjun, Sebastian Reichel, linux-pm@vger.kernel.org
Hi Wei,
On Fri, May 19, 2017 at 01:27:51AM +0000, weiyongjun (A) wrote:
> Hi, Dmitry
>
> > Subject: Re: [PATCH -next] power: supply: ltc3651-charger: Fix wrong pointer
> > passed to PTR_ERR()
> >
> > On Wed, May 17, 2017 at 12:11:32PM +0000, Wei Yongjun wrote:
> > > From: Wei Yongjun <weiyongjun1@huawei.com>
> > >
> > > PTR_ERR should access the value just tested by IS_ERR, otherwise
> > > the wrong error code will be returned.
> > >
> > > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> >
> > This does fix the issue, but maybe we want to use PTR_ERR_OR_ZERO()?
> >
> > ltc3651_charger->acpr_gpio = devm_gpiod_get(&pdev->dev,
> > "lltc,acpr", GPIOD_IN);
> > ret = PTR_ERR_OR_ZERO(ltc3651_charger->acpr_gpio);
> > if (ret)
> > ...
> >
> > Or even
> >
> > err = PTR_ERR_OR_ZERO((ltc3651_charger->acpr_gpio =
> > devm_gpiod_get(&pdev->dev, "lltc,acpr", GPIOD_IN)));
> > if (err)
> > ...
> >
> > To make sure we never mix up pointer. Maybe we need
> > ERR_OR_ASSIGN(ptr1,
> > ptr2) to avoid the ugly assignment in argument above...
> >
>
> I have checked devm_gpiod_get() and seems it never return NULL. And the other
> places call devm_gpiod_get() never check for NULL return. So I think PTR_ERR_OR_ZERO()
> is not need here.
The request to use PTR_ERR_OR_ZERO() had nothing to do with
devm_gpiod_get() returning NULL and everything with reducing number of
times one has to write "ltc3651_charger->acpr_gpio". The original
problem was that we checked the wrong value, if we provide macro
incorporating assignment and conversion to an error code in one step
this will reduce number of times we screw up like that in the future.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH -next] power: supply: ltc3651-charger: Fix wrong pointer passed to PTR_ERR()
2017-05-17 12:11 [PATCH -next] power: supply: ltc3651-charger: Fix wrong pointer passed to PTR_ERR() Wei Yongjun
2017-05-19 0:59 ` Dmitry Torokhov
@ 2017-06-07 20:16 ` Sebastian Reichel
1 sibling, 0 replies; 5+ messages in thread
From: Sebastian Reichel @ 2017-06-07 20:16 UTC (permalink / raw)
To: Wei Yongjun; +Cc: Dmitry Torokhov, Wei Yongjun, linux-pm
[-- Attachment #1: Type: text/plain, Size: 361 bytes --]
Hi,
On Wed, May 17, 2017 at 12:11:32PM +0000, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> PTR_ERR should access the value just tested by IS_ERR, otherwise
> the wrong error code will be returned.
I queued the same patch from Dan, which had the better description
incl. a Fixes, but added your Signed-off-by.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-06-07 20:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-17 12:11 [PATCH -next] power: supply: ltc3651-charger: Fix wrong pointer passed to PTR_ERR() Wei Yongjun
2017-05-19 0:59 ` Dmitry Torokhov
2017-05-19 1:27 ` weiyongjun (A)
2017-05-19 22:07 ` Dmitry Torokhov
2017-06-07 20:16 ` Sebastian Reichel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).