On Thu, Sep 10, 2020 at 3:30 AM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> 0-day rightfully complains about a sometimes uninitialized variable
> in pmbus_get_boolean().
>
> drivers/hwmon/pmbus/pmbus_core.c:903:13: warning:
> variable 'ret' is used uninitialized whenever 'if' condition is true
> } else if (!s1 || !s2) {
>
> While that is technically true, it won't be hit in the field since the
> condition indicates a programming error. Move the check of that condition
> into the code generating the attribute entry, and refuse generating the
> attribute if the condition is true. Swap the condition check in
> pmbus_get_boolean() to ensure that static analyzers don't get a hiccup
> (because we check if s1 and s2 are NULL, static analyzers may believe
> that they can be NULL independently of each other).
>
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Cc: Alex Qiu <xqiu@xxxxxxxxxx>
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
Reviewed-by: Alex Qiu <xqiu@xxxxxxxxxx>
> ---
> drivers/hwmon/pmbus/pmbus_core.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 5113b5eca17a..6be08696189c 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -898,11 +898,7 @@ static int pmbus_get_boolean(struct i2c_client *client, struct pmbus_boolean *b,
> pmbus_update_sensor_data(client, s2);
>
> regval = status & mask;
> - if (!s1 && !s2) {
> - ret = !!regval;
> - } else if (!s1 || !s2) {
> - WARN(1, "Bad boolean descriptor %p: s1=%p, s2=%p\n", b, s1, s2);
> - } else {
> + if (s1 && s2) {
> s64 v1, v2;
>
> if (s1->data < 0) {
> @@ -917,6 +913,8 @@ static int pmbus_get_boolean(struct i2c_client *client, struct pmbus_boolean *b,
> v1 = pmbus_reg2data(data, s1);
> v2 = pmbus_reg2data(data, s2);
> ret = !!(regval && v1 >= v2);
> + } else {
> + ret = !!regval;
> }
> unlock:
> mutex_unlock(&data->update_lock);
> @@ -1044,6 +1042,9 @@ static int pmbus_add_boolean(struct pmbus_data *data,
> struct pmbus_boolean *boolean;
> struct sensor_device_attribute *a;
>
> + if (WARN((s1 && !s2) || (!s1 && s2), "Bad s1/s2 parameters\n"))
> + return -EINVAL;
> +
> boolean = devm_kzalloc(data->dev, sizeof(*boolean), GFP_KERNEL);
> if (!boolean)
> return -ENOMEM;
> --
> 2.17.1
>
Thanks!
- Alex Qiu
