On Fri, Aug 19, 2011 at 03:07:56AM -0400, Jean Delvare wrote:
> I am under the impression that error paths in functions
> aem_init_aem1_inst() and aem_init_aem2_inst() are incorrect. In
> several cases, the function returns 0 on error, which I suspect is
> not intended. Fix this by properly tracking error codes.
>
> Signed-off-by: Jean Delvare <khali@xxxxxxxxxxxx>
> Cc: Darrick J. Wong <djwong@xxxxxxxxxx>
> Cc: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx>
> ---
> Darrick, can you please confirm that none of the missing error codes
> was on purpose, and that my patch doesn't break anything? Thanks.
>
Jean,
with your new code, if instance <n> fails, subsequent instances will not be initialized.
Already initialized instances will not be removed. So you end up with instances
0 .. <n-1>. In the old code, it was possible to end up with instances 0 .. <n-1> <n+1> ... <m>.
So there is definitely a behavioral change.
On the other side, it is odd that some errors would cause instantiation to abort, while
instantiation would continue for other errors.
Guenter
> drivers/hwmon/ibmaem.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> --- linux-3.1-rc2.orig/drivers/hwmon/ibmaem.c 2011-08-18 18:57:17.000000000 +0200
> +++ linux-3.1-rc2/drivers/hwmon/ibmaem.c 2011-08-18 19:16:12.000000000 +0200
> @@ -602,24 +602,26 @@ static int aem_init_aem1_inst(struct aem
> platform_set_drvdata(data->pdev, data);
>
> /* Set up IPMI interface */
> - if (aem_init_ipmi_data(&data->ipmi, probe->interface,
> - probe->bmc_device))
> + res = aem_init_ipmi_data(&data->ipmi, probe->interface,
> + probe->bmc_device);
> + if (res)
> goto ipmi_err;
>
> /* Register with hwmon */
> data->hwmon_dev = hwmon_device_register(&data->pdev->dev);
> -
> if (IS_ERR(data->hwmon_dev)) {
> dev_err(&data->pdev->dev, "Unable to register hwmon "
> "device for IPMI interface %d\n",
> probe->interface);
> + res = PTR_ERR(data->hwmon_dev);
> goto hwmon_reg_err;
> }
>
> data->update = update_aem1_sensors;
>
> /* Find sensors */
> - if (aem1_find_sensors(data))
> + res = aem1_find_sensors(data);
> + if (res)
> goto sensor_err;
>
> /* Add to our list of AEM devices */
> @@ -735,24 +737,26 @@ static int aem_init_aem2_inst(struct aem
> platform_set_drvdata(data->pdev, data);
>
> /* Set up IPMI interface */
> - if (aem_init_ipmi_data(&data->ipmi, probe->interface,
> - probe->bmc_device))
> + res = aem_init_ipmi_data(&data->ipmi, probe->interface,
> + probe->bmc_device);
> + if (res)
> goto ipmi_err;
>
> /* Register with hwmon */
> data->hwmon_dev = hwmon_device_register(&data->pdev->dev);
> -
> if (IS_ERR(data->hwmon_dev)) {
> dev_err(&data->pdev->dev, "Unable to register hwmon "
> "device for IPMI interface %d\n",
> probe->interface);
> + res = PTR_ERR(data->hwmon_dev);
> goto hwmon_reg_err;
> }
>
> data->update = update_aem2_sensors;
>
> /* Find sensors */
> - if (aem2_find_sensors(data))
> + res = aem2_find_sensors(data);
> + if (res)
> goto sensor_err;
>
> /* Add to our list of AEM devices */
>
>
> --
> Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
