[PATCH] regulator: core: Use IS_ERR_OR_NULL()

[PATCH] regulator: core: Use IS_ERR_OR_NULL()

[Viresh Kumar]

Use IS_ERR_OR_NULL() rather than open coding it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/regulator/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 78387a6cbae5..55b49acfd9b3 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1595,7 +1595,7 @@ static void _regulator_put(struct regulator *regulator)
 {
 	struct regulator_dev *rdev;
 
-	if (regulator == NULL || IS_ERR(regulator))
+	if (IS_ERR_OR_NULL(regulator))
 		return;
 
 	rdev = regulator->rdev;
-- 
2.4.0

Re: [PATCH] regulator: core: Use IS_ERR_OR_NULL()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 264 bytes --]

On Mon, Aug 17, 2015 at 12:30:58PM +0530, Viresh Kumar wrote:
> Use IS_ERR_OR_NULL() rather than open coding it.

Neither of the patches you sent today applied cleanly against current
code, please submit patches against the tree they're intended to be
applied on.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] regulator: core: Use IS_ERR_OR_NULL()

[Viresh Kumar]

On 17-08-15, 13:06, Mark Brown wrote:
> On Mon, Aug 17, 2015 at 12:30:58PM +0530, Viresh Kumar wrote:
> > Use IS_ERR_OR_NULL() rather than open coding it.
> 
> Neither of the patches you sent today applied cleanly against current
> code, please submit patches against the tree they're intended to be
> applied on.

That's the last I wanted to get ranted about, Sigh.

I thought, my base is the latest linux-next/master, which will have
your latest updates. But that wasn't the case. I was sitting on a
slightly older base. Sorry about that.

Will base out of your tree directly in future.

-- 
viresh

Re: [PATCH] regulator: core: Use IS_ERR_OR_NULL()

[Arnd Bergmann]

On Monday 17 August 2015 12:30:58 Viresh Kumar wrote:
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 78387a6cbae5..55b49acfd9b3 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -1595,7 +1595,7 @@ static void _regulator_put(struct regulator *regulator)
>  {
>         struct regulator_dev *rdev;
>  
> -       if (regulator == NULL || IS_ERR(regulator))
> +       if (IS_ERR_OR_NULL(regulator))
>                 return;

The use of IS_ERR_OR_NULL is almost always a bug (as is the open-coded
equivalent).
Please try to find out why this is done here and add a comment
if it's actually correct, or fix it if not.

My guess is that it is not ok to pass an error pointer as the argument
of _regulator_put(), so maybe there should be WARN_ON()?

	Arnd

Re: [PATCH] regulator: core: Use IS_ERR_OR_NULL()

[Viresh Kumar]

On 20-08-15, 15:26, Arnd Bergmann wrote:
> The use of IS_ERR_OR_NULL is almost always a bug (as is the open-coded
> equivalent).
> Please try to find out why this is done here and add a comment
> if it's actually correct, or fix it if not.
> 
> My guess is that it is not ok to pass an error pointer as the argument
> of _regulator_put(), so maybe there should be WARN_ON()?

I agree, but updating that would require a revisit of all the API
users, in case they are passing a possible error value to it.

I am fine at attempting to fix that, but would need an Ack from Mark
first.

-- 
viresh