Re: [PATCH] hwmon: (w83781d) Fix compile warning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 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


[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux