* [PATCH] regmap: Fix compile warning with value uninitialized
@ 2013-11-09 9:49 Caizhiyong
2013-11-09 15:16 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Caizhiyong @ 2013-11-09 9:49 UTC (permalink / raw)
To: Mark Brown
Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
Wanglin (Albert)
From: Cai Zhiyong <caizhiyong@huawei.com>
Date: Sat, 9 Nov 2013 14:14:36 +0800
Subject: [PATCH] regmap: Fix compile warning with value uninitialized
Fix compile warning with value uninitialized:
drivers/base/regmap/regmap.c:2173: warning: 'ret' may be used uninitialized in this function
Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
---
drivers/base/regmap/regmap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 9c021d9..4fac073 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2170,7 +2170,8 @@ 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;
map->lock(map->lock_arg);
--
1.8.1.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] regmap: Fix compile warning with value uninitialized
2013-11-09 9:49 [PATCH] regmap: Fix compile warning with value uninitialized Caizhiyong
@ 2013-11-09 15:16 ` Mark Brown
2013-11-09 15:34 ` Levente Kurusa
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2013-11-09 15:16 UTC (permalink / raw)
To: Caizhiyong
Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
Wanglin (Albert)
[-- Attachment #1: Type: text/plain, Size: 492 bytes --]
On Sat, Nov 09, 2013 at 09:49:11AM +0000, Caizhiyong wrote:
> @@ -2170,7 +2170,8 @@ 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;
This sort of fix isn't ideal, it just silences the warning but if the
compiler has spotted a genuine oversight in the function it won't
address it. It's better to include some analysis of why this is a good
fix.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] regmap: Fix compile warning with value uninitialized
2013-11-09 15:16 ` Mark Brown
@ 2013-11-09 15:34 ` Levente Kurusa
2013-11-09 16:15 ` Greg Kroah-Hartman
2013-11-09 18:57 ` Mark Brown
0 siblings, 2 replies; 6+ messages in thread
From: Levente Kurusa @ 2013-11-09 15:34 UTC (permalink / raw)
To: Mark Brown, Caizhiyong
Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
Wanglin (Albert)
2013-11-09 16:16 keltezéssel, Mark Brown írta:
> On Sat, Nov 09, 2013 at 09:49:11AM +0000, Caizhiyong wrote:
>
>> @@ -2170,7 +2170,8 @@ 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;
Wouldn't the following be better?
int i, ret = 0;
I think it is more readable.
>
> This sort of fix isn't ideal, it just silences the warning but if the
> compiler has spotted a genuine oversight in the function it won't
> address it. It's better to include some analysis of why this is a good
> fix.
>
The only condition when 'ret' is not set is when the num_regs parameter is zero
and krealloc doesn't fail. If the above two conditions apply, 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.
Also, I think code which throws warnings is worse than code
which doesn't.
--
Regards,
Levente Kurusa
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] regmap: Fix compile warning with value uninitialized
2013-11-09 15:34 ` Levente Kurusa
@ 2013-11-09 16:15 ` Greg Kroah-Hartman
2013-11-09 18:57 ` Mark Brown
1 sibling, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-09 16:15 UTC (permalink / raw)
To: Levente Kurusa
Cc: Mark Brown, Caizhiyong, linux-kernel@vger.kernel.org,
Wanglin (Albert)
On Sat, Nov 09, 2013 at 04:34:07PM +0100, Levente Kurusa wrote:
> 2013-11-09 16:16 keltezéssel, Mark Brown írta:
> > On Sat, Nov 09, 2013 at 09:49:11AM +0000, Caizhiyong wrote:
> >
> >> @@ -2170,7 +2170,8 @@ 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;
> Wouldn't the following be better?
>
> int i, ret = 0;
>
> I think it is more readable.
No, please put them on separate lines like the patch did.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] regmap: Fix compile warning with value uninitialized
2013-11-09 15:34 ` Levente Kurusa
2013-11-09 16:15 ` Greg Kroah-Hartman
@ 2013-11-09 18:57 ` Mark Brown
2013-11-09 19:02 ` Levente Kurusa
1 sibling, 1 reply; 6+ messages in thread
From: Mark Brown @ 2013-11-09 18:57 UTC (permalink / raw)
To: Levente Kurusa
Cc: Caizhiyong, Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
Wanglin (Albert)
[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]
On Sat, Nov 09, 2013 at 04:34:07PM +0100, Levente Kurusa wrote:
> 2013-11-09 16:16 keltezéssel, Mark Brown írta:
> >> - int i, ret;
> >> + int i;
> >> + int ret = 0;
> >> bool bypass;
> Wouldn't the following be better?
> int i, ret = 0;
> I think it is more readable.
No, that's not the kernel coding style.
> > This sort of fix isn't ideal, it just silences the warning but if the
> > compiler has spotted a genuine oversight in the function it won't
> > address it. It's better to include some analysis of why this is a good
> > fix.
> The only condition when 'ret' is not set is when the num_regs parameter is zero
> and krealloc doesn't fail. If the above two conditions apply, 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.
OK, so that should be in the changelog - or there should be an error
check for num_regs == 0 which might be more helpful.
> Also, I think code which throws warnings is worse than code
> which doesn't.
Right, but code that is behaves incorrectly is worse still. We need to
be careful that when changing the code to remove warnings we make sure
that we fix any real problems the compiler was warning us about.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regmap: Fix compile warning with value uninitialized
2013-11-09 18:57 ` Mark Brown
@ 2013-11-09 19:02 ` Levente Kurusa
0 siblings, 0 replies; 6+ messages in thread
From: Levente Kurusa @ 2013-11-09 19:02 UTC (permalink / raw)
To: Mark Brown, Caizhiyong
Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
Wanglin (Albert)
2013-11-09 19:57 keltezéssel, Mark Brown írta:
> On Sat, Nov 09, 2013 at 04:34:07PM +0100, Levente Kurusa wrote:
>> 2013-11-09 16:16 keltezéssel, Mark Brown írta:
>
>>>> - int i, ret;
>>>> + int i;
>>>> + int ret = 0;
>>>> bool bypass;
>
>> Wouldn't the following be better?
>
>> int i, ret = 0;
>
>> I think it is more readable.
>
> No, that's not the kernel coding style.
Alright.
>
>>> This sort of fix isn't ideal, it just silences the warning but if the
>>> compiler has spotted a genuine oversight in the function it won't
>>> address it. It's better to include some analysis of why this is a good
>>> fix.
>
>> The only condition when 'ret' is not set is when the num_regs parameter is zero
>> and krealloc doesn't fail. If the above two conditions apply, 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.
>
> OK, so that should be in the changelog - or there should be an error
> check for num_regs == 0 which might be more helpful.
Adding an explanation into the changelog and editing the patch
in a way so that it adds a check would be the best choice I think.
Cai, can you please do this as part of the patch?
--
Regards,
Levente Kurusa
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-09 19:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-09 9:49 [PATCH] regmap: Fix compile warning with value uninitialized Caizhiyong
2013-11-09 15:16 ` Mark Brown
2013-11-09 15:34 ` Levente Kurusa
2013-11-09 16:15 ` Greg Kroah-Hartman
2013-11-09 18:57 ` Mark Brown
2013-11-09 19:02 ` Levente Kurusa
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).