[PATCH] pwm: fix used-uninitialized warning in pwm_get()

From: Stephen Warren <[email protected]>

This fixes:
drivers/pwm/core.c: In function 'pwm_get':
drivers/pwm/core.c:534:15: warning: 'index' may be used uninitialized in this function

The addition to the if condition at end of the function isn't strictly
necessary to solve the warning, but does make it more obvious that the
initialization of "index" to a dummy value isn't just hiding the
problem.

Signed-off-by: Stephen Warren <[email protected]>
---
drivers/pwm/core.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index dbab530..2b401c0 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -531,7 +531,7 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
struct pwm_chip *chip = NULL;
unsigned int best = 0;
struct pwm_lookup *p;
- unsigned int index;
+ unsigned int index = -1;
unsigned int match;

/* look up via DT first */
@@ -588,7 +588,7 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
}
}

- if (chip)
+ if (chip && index != -1)
pwm = pwm_request_from_chip(chip, index, con_id ?: dev_id);

mutex_unlock(&pwm_lookup_lock);
--
1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On Tue, Jul 03, 2012 at 02:34:21PM -0600, Stephen Warren wrote:
> From: Stephen Warren <[email protected]>
>
> This fixes:
> drivers/pwm/core.c: In function 'pwm_get':
> drivers/pwm/core.c:534:15: warning: 'index' may be used uninitialized in this function
>
> The addition to the if condition at end of the function isn't strictly
> necessary to solve the warning, but does make it more obvious that the
> initialization of "index" to a dummy value isn't just hiding the
> problem.

Actually this seems to be a false positive, and one that I don't see (I
use GCC 4.6.3). index will be initialized when chip is set in the loop.
My guess is that GCC 4.6.3 actually notices while your version doesn't.

Thierry

> Signed-off-by: Stephen Warren <[email protected]>
> ---
> drivers/pwm/core.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index dbab530..2b401c0 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -531,7 +531,7 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
> struct pwm_chip *chip = NULL;
> unsigned int best = 0;
> struct pwm_lookup *p;
> - unsigned int index;
> + unsigned int index = -1;
> unsigned int match;
>
> /* look up via DT first */
> @@ -588,7 +588,7 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
> }
> }
>
> - if (chip)
> + if (chip && index != -1)
> pwm = pwm_request_from_chip(chip, index, con_id ?: dev_id);
>
> mutex_unlock(&pwm_lookup_lock);
> --
> 1.7.0.4
>
>
>

On 07/03/2012 11:58 PM, Thierry Reding wrote:
> On Tue, Jul 03, 2012 at 02:34:21PM -0600, Stephen Warren wrote:
>> From: Stephen Warren <[email protected]>
>>
>> This fixes: drivers/pwm/core.c: In function 'pwm_get':
>> drivers/pwm/core.c:534:15: warning: 'index' may be used
>> uninitialized in this function
>>
>> The addition to the if condition at end of the function isn't
>> strictly necessary to solve the warning, but does make it more
>> obvious that the initialization of "index" to a dummy value isn't
>> just hiding the problem.
>
> Actually this seems to be a false positive, and one that I don't
> see (I use GCC 4.6.3). index will be initialized when chip is set
> in the loop. My guess is that GCC 4.6.3 actually notices while your
> version doesn't.

Yes, it is a false-positive, which is why I was fine with just
initializing the variable to hide the warning rather than making some
other code change. I think there's still value in hiding the warning
though, so that:

a) Nobody else has to look at the warning and decide it's a false
positive and remember to ignore it.

b) The fewer warnings there are, the more likely new warnings will be
noticed and analyzed.

So I'd still argue for this change, or some other fix for the warning,
be merged.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On Thu, Jul 05, 2012 at 09:56:50AM -0600, Stephen Warren wrote:
> On 07/03/2012 11:58 PM, Thierry Reding wrote:
> > On Tue, Jul 03, 2012 at 02:34:21PM -0600, Stephen Warren wrote:
> >> From: Stephen Warren <[email protected]>
> >>
> >> This fixes: drivers/pwm/core.c: In function 'pwm_get':
> >> drivers/pwm/core.c:534:15: warning: 'index' may be used
> >> uninitialized in this function
> >>
> >> The addition to the if condition at end of the function isn't
> >> strictly necessary to solve the warning, but does make it more
> >> obvious that the initialization of "index" to a dummy value isn't
> >> just hiding the problem.
> >
> > Actually this seems to be a false positive, and one that I don't
> > see (I use GCC 4.6.3). index will be initialized when chip is set
> > in the loop. My guess is that GCC 4.6.3 actually notices while your
> > version doesn't.
>
> Yes, it is a false-positive, which is why I was fine with just
> initializing the variable to hide the warning rather than making some
> other code change. I think there's still value in hiding the warning
> though, so that:
>
> a) Nobody else has to look at the warning and decide it's a false
> positive and remember to ignore it.
>
> b) The fewer warnings there are, the more likely new warnings will be
> noticed and analyzed.
>
> So I'd still argue for this change, or some other fix for the warning,
> be merged.

Okay, I've applied a patch based on what you did, with a more explicit
description of what's going on. Since I didn't see the warning with my
toolchain before it would be good if you could retest and verify that it
indeed fixes the problem for you.

Thierry