On Mon, Jun 18, 2012 at 12:07:25PM +0200, Jean Delvare wrote:
> Hi Guenter,
>
> On Wed, 13 Jun 2012 13:56:26 -0700, Guenter Roeck wrote:
> > The following compile warning may be seen if the driver is compiled with
> > -Wuninitialized:
> >
> > drivers/hwmon/w83781d.c: warning: 'sc_addr[1]' may be used uninitialized in this
> > function [-Wuninitialized]
> >
> > While this is a false positive, it is annoying in nightly builds, and may help
> > to conceal real problems. The current code is quite tricky, and and it is easy
> > to rearrage the code to make the problem disappear. So fix it.
>
> I don't see this warning here (gcc 4.6.2.) While I see why it can be
> reported, I don't really understand why your change makes it disappear,
> as you still depend on the value of a local variable, which could have
> changed between the moment you set sc_addr[1] and the moment you use
> it. I'm curious why the compiler is able to keep track of the value in
> one case and not in the other case.
>
> Out of curiosity, have you tried defining kind as const? If it makes
> the compiler equally happy, I think it would be a better way to solve
> this warning.
>
Hi Jean,
did you have time to look into this some more ? Statistically, I see the warning
in about one out of ten randconfig builds.
Thanks,
Guenter
> > Cc: Jean Delvare <khali@xxxxxxxxxxxx>
> > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> > ---
> > drivers/hwmon/w83781d.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hwmon/w83781d.c b/drivers/hwmon/w83781d.c
> > index b03d54a..a82f508 100644
> > --- a/drivers/hwmon/w83781d.c
> > +++ b/drivers/hwmon/w83781d.c
> > @@ -867,6 +867,7 @@ w83781d_detect_subclients(struct i2c_client *new_client)
> > struct i2c_adapter *adapter = new_client->adapter;
> > struct w83781d_data *data = i2c_get_clientdata(new_client);
> > enum chips kind = data->type;
> > + int num_adapter = 1;
>
> This name is inadequate, you're not counting adapters but subclients.
> num_sc maybe? Or num_subclients if you want to be verbose.
>
> >
> > id = i2c_adapter_id(adapter);
> >
> > @@ -891,6 +892,7 @@ w83781d_detect_subclients(struct i2c_client *new_client)
> > }
> >
> > if (kind != w83783s) {
> > + num_adapter = 2;
> > if (force_subclients[0] == id &&
> > force_subclients[1] == address) {
> > sc_addr[1] = force_subclients[3];
> > @@ -906,7 +908,7 @@ w83781d_detect_subclients(struct i2c_client *new_client)
> > }
> > }
> >
> > - for (i = 0; i <= 1; i++) {
> > + for (i = 0; i < num_adapter; i++) {
> > data->lm75[i] = i2c_new_dummy(adapter, sc_addr[i]);
> > if (!data->lm75[i]) {
> > dev_err(&new_client->dev, "Subclient %d "
> > @@ -917,8 +919,6 @@ w83781d_detect_subclients(struct i2c_client *new_client)
> > goto ERROR_SC_3;
> > goto ERROR_SC_2;
> > }
> > - if (kind == w83783s)
> > - break;
> > }
> >
> > return 0;
>
>
> --
> Jean Delvare
>
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
