[PATCH] regmap: Fix 'ret' would return an uninitialized value

[PATCH] regmap: Fix 'ret' would return an uninitialized value

[Caizhiyong]

From: Cai Zhiyong <caizhiyong@huawei.com>
Date: Mon, 18 Nov 2013 20:21:49 +0800
Subject: [PATCH] regmap: Fix 'ret' would return an uninitialized value

This patch give a warning when calling regmap_register_patch with
parameter num_regs <= 0.

When the num_regs parameter is zero and krealloc doesn't fail,
then the code would return an uninitialized value. However,
calling this function with num_regs == 0, would be a waste as it
essentially does nothing.

Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
---
 drivers/base/regmap/regmap.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 9c021d9..9a36ac1 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2173,6 +2173,10 @@ int regmap_register_patch(struct regmap *map, const struct reg_default *regs,
 	int i, ret;
 	bool bypass;
 
+	if (WARN_ONCE(num_regs <= 0, "invalid registers number (%d)\n",
+	    num_regs))
+		return 0;
+
 	map->lock(map->lock_arg);
 
 	bypass = map->cache_bypass;
-- 
1.8.1.5

Re: [PATCH] regmap: Fix 'ret' would return an uninitialized value

[Mark Brown]

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

On Mon, Nov 18, 2013 at 12:29:02PM +0000, Caizhiyong wrote:

> This patch give a warning when calling regmap_register_patch with
> parameter num_regs <= 0.

Applied, thanks.

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

Re: [PATCH] regmap: Fix 'ret' would return an uninitialized value

[Geyslan Gregório Bem]

2013/11/18 Caizhiyong <caizhiyong@hisilicon.com>:
> From: Cai Zhiyong <caizhiyong@huawei.com>
> Date: Mon, 18 Nov 2013 20:21:49 +0800
> Subject: [PATCH] regmap: Fix 'ret' would return an uninitialized value
>
> This patch give a warning when calling regmap_register_patch with
> parameter num_regs <= 0.
>
> When the num_regs parameter is zero and krealloc doesn't fail,
> then the code would return an uninitialized value. However,
> calling this function with num_regs == 0, would be a waste as it
> essentially does nothing.
>
> Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>

Reviewed-by: Geyslan G. Bem <geyslan@gmail.com>

[PATCH] regmap: Fix 'ret' would return an uninitialized value

[Caizhiyong]

From: Cai Zhiyong <caizhiyong@huawei.com>
Date: Mon, 11 Nov 2013 19:26:14 +0800
Subject: [PATCH] regmap: Fix 'ret' would return an uninitialized value

This patch give a warning when calling regmap_register_patch with
parameter num_regs <= 0.

When the num_regs parameter is zero and krealloc doesn't fail,
then the code would return an uninitialized value. However,
calling this function with num_regs == 0, would be a waste as it
essentially does nothing.

Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
---
 drivers/base/regmap/regmap.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 9c021d9..7b5c28a 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2170,9 +2170,12 @@ int regmap_register_patch(struct regmap *map, const struct reg_default *regs,
 			  int num_regs)
 {
 	struct reg_default *p;
 	int i, ret;
 	bool bypass;
 
+	if (WARN_ONCE(num_regs <= 0))
+		return 0;
+
 	map->lock(map->lock_arg);
 
 	bypass = map->cache_bypass;
-- 
1.8.1.5

Re: [PATCH] regmap: Fix 'ret' would return an uninitialized value

[Geyslan Gregório Bem]

2013/11/15 Caizhiyong <caizhiyong@hisilicon.com>:
> From: Cai Zhiyong <caizhiyong@huawei.com>
> Date: Mon, 11 Nov 2013 19:26:14 +0800
> Subject: [PATCH] regmap: Fix 'ret' would return an uninitialized value
>
> This patch give a warning when calling regmap_register_patch with
> parameter num_regs <= 0.
>
> When the num_regs parameter is zero and krealloc doesn't fail,
> then the code would return an uninitialized value. However,
> calling this function with num_regs == 0, would be a waste as it
> essentially does nothing.
>
> Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
Reviewed-by: Geyslan G. Bem <geyslan@gmail.com>

Seems a good approach.

Re: [PATCH] regmap: Fix 'ret' would return an uninitialized value

[Levente Kurusa]

2013-11-15 14:00 keltezéssel, Geyslan Gregório Bem írta:
> 2013/11/15 Caizhiyong <caizhiyong@hisilicon.com>:
>> From: Cai Zhiyong <caizhiyong@huawei.com>
>> Date: Mon, 11 Nov 2013 19:26:14 +0800
>> Subject: [PATCH] regmap: Fix 'ret' would return an uninitialized value
>>
>> This patch give a warning when calling regmap_register_patch with
>> parameter num_regs <= 0.
>>
>> When the num_regs parameter is zero and krealloc doesn't fail,
>> then the code would return an uninitialized value. However,
>> calling this function with num_regs == 0, would be a waste as it
>> essentially does nothing.
>>
>> Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
> Reviewed-by: Geyslan G. Bem <geyslan@gmail.com>
> 
> Seems a good approach.
> 

The patch doesn't apply. After manually applying this it
threw a compilation error:

drivers/base/regmap/regmap.c: In function ‘regmap_register_patch’:
drivers/base/regmap/regmap.c:2176:6: error: expected expression before ‘)’ token


Cai,
You have missed the second parameter of WARN_ONCE().
Please append what you'd prefer to have outputten once the condition evaluates to true.

-- 
Regards,
Levente Kurusa

Re: [PATCH] regmap: Fix 'ret' would return an uninitialized value

[Geyslan Gregório Bem]

2013/11/15 Levente Kurusa <levex@linux.com>:
> 2013-11-15 14:00 keltezéssel, Geyslan Gregório Bem írta:
>> 2013/11/15 Caizhiyong <caizhiyong@hisilicon.com>:
>>> From: Cai Zhiyong <caizhiyong@huawei.com>
>>> Date: Mon, 11 Nov 2013 19:26:14 +0800
>>> Subject: [PATCH] regmap: Fix 'ret' would return an uninitialized value
>>>
>>> This patch give a warning when calling regmap_register_patch with
>>> parameter num_regs <= 0.
>>>
>>> When the num_regs parameter is zero and krealloc doesn't fail,
>>> then the code would return an uninitialized value. However,
>>> calling this function with num_regs == 0, would be a waste as it
>>> essentially does nothing.
>>>
>>> Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
>> Reviewed-by: Geyslan G. Bem <geyslan@gmail.com>
>>
>> Seems a good approach.
>>
>
> The patch doesn't apply. After manually applying this it
> threw a compilation error:
>
> drivers/base/regmap/regmap.c: In function ‘regmap_register_patch’:
> drivers/base/regmap/regmap.c:2176:6: error: expected expression before ‘)’ token
>
>
> Cai,
> You have missed the second parameter of WARN_ONCE().
> Please append what you'd prefer to have outputten once the condition evaluates to true.
>
> --
> Regards,
> Levente Kurusa
Cai, Levente,

Maybe this:

+       if (WARN_ONCE(num_regs <= 0, "number of registers [%d] must be
major than 0", num_regs))
+               return 0;
+

-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH] regmap: Fix 'ret' would return an uninitialized value

[Mark Brown]

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

On Fri, Nov 15, 2013 at 01:52:19PM -0200, Geyslan Gregório Bem wrote:

> +       if (WARN_ONCE(num_regs <= 0, "number of registers [%d] must be
> major than 0", num_regs))
> +               return 0;

"must be more than" but yes.

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

Re: [PATCH] regmap: Fix 'ret' would return an uninitialized value

[Geyslan Gregório Bem]

2013/11/15 Mark Brown <broonie@kernel.org>:
> On Fri, Nov 15, 2013 at 01:52:19PM -0200, Geyslan Gregório Bem wrote:
>
>> +       if (WARN_ONCE(num_regs <= 0, "number of registers [%d] must be
>> major than 0", num_regs))
>> +               return 0;
>
> "must be more than" but yes.

Tks. Sorry for my poor english. :D

-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH] regmap: Fix 'ret' would return an uninitialized value

[Levente Kurusa]

2013-11-15 16:52 keltezéssel, Geyslan Gregório Bem írta:
> 2013/11/15 Levente Kurusa <levex@linux.com>:
>> 2013-11-15 14:00 keltezéssel, Geyslan Gregório Bem írta:
>>> 2013/11/15 Caizhiyong <caizhiyong@hisilicon.com>:
>>>> From: Cai Zhiyong <caizhiyong@huawei.com>
>>>> Date: Mon, 11 Nov 2013 19:26:14 +0800
>>>> Subject: [PATCH] regmap: Fix 'ret' would return an uninitialized value
>>>>
>>>> This patch give a warning when calling regmap_register_patch with
>>>> parameter num_regs <= 0.
>>>>
>>>> When the num_regs parameter is zero and krealloc doesn't fail,
>>>> then the code would return an uninitialized value. However,
>>>> calling this function with num_regs == 0, would be a waste as it
>>>> essentially does nothing.
>>>>
>>>> Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
>>> Reviewed-by: Geyslan G. Bem <geyslan@gmail.com>
>>>
>>> Seems a good approach.
>>>
>>
>> The patch doesn't apply. After manually applying this it
>> threw a compilation error:
>>
>> drivers/base/regmap/regmap.c: In function ‘regmap_register_patch’:
>> drivers/base/regmap/regmap.c:2176:6: error: expected expression before ‘)’ token
>>
>>
>> Cai,
>> You have missed the second parameter of WARN_ONCE().
>> Please append what you'd prefer to have outputten once the condition evaluates to true.
>>
>> --
>> Regards,
>> Levente Kurusa
> Cai, Levente,
> 
> Maybe this:
> 
> +       if (WARN_ONCE(num_regs <= 0, "number of registers [%d] must be
> major than 0", num_regs))
> +               return 0;
> +
> 

It is fine, but I think outputting num_regs is unnecessary.
Also, that line is more than 80 characters, so please wrap it.

-- 
Regards,
Levente Kurusa

[PATCH] regmap: Fix 'ret' would return an uninitialized value

[Caizhiyong]

From: Cai Zhiyong <caizhiyong@huawei.com>
Date: Mon, 11 Nov 2013 19:26:14 +0800
Subject: [PATCH] regmap: Fix 'ret' would return an uninitialized value

 - Fix 'ret' would return an uninitialized value.
 - Add a warning avoid invalid 'num_regs' value passed in.

When the num_regs parameter is zero and krealloc doesn't fail,
then the code would return an uninitialized value. However,
calling this function with num_regs == 0, would be a waste as it
essentially does nothing.

Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
---
 drivers/base/regmap/regmap.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 9c021d9..7b5c28a 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2170,9 +2170,15 @@ int regmap_register_patch(struct regmap *map, const struct reg_default *regs,
 			  int num_regs)
 {
 	struct reg_default *p;
-	int i, ret;
+	int i;
+	int ret = 0;
 	bool bypass;
 
+	if (num_regs <= 0) {
+		WARN_ONCE(1, "Call regmap_register_patch with num_regs <= 0.");
+		return 0;
+	}
+
 	map->lock(map->lock_arg);
 
 	bypass = map->cache_bypass;
-- 
1.8.1.5

Re: [PATCH] regmap: Fix 'ret' would return an uninitialized value

[Mark Brown]

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

On Mon, Nov 11, 2013 at 11:35:52AM +0000, Caizhiyong wrote:

> +	int ret = 0;

With the if statement below you probably don't need to do this any more.

> +	if (num_regs <= 0) {
> +		WARN_ONCE(1, "Call regmap_register_patch with num_regs <= 0.");
> +		return 0;
> +	}

WARN_ONCE actually returns the result of the test so you should be able
to write this as:

	if (WARN_ONCE(num_regs <= 0))
		return 0;

which is slightly neater.

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