Hi Hans, Jeff,
> You (Jean) already mailed me about this and it was on my todo list,
> but I'm currently rather busy with work. So it looks like Jeff beat
> me to it.
>
> Jeff's patch looks fine, please apply. Thanks Jeff!
The patch isn't wrong per se, but it could be made more simple, and is
incomplete in comparison to what was done for all other hardware
monitoring drivers:
* We want to create all the files before registering with the hwmon
class, this closes a race condition.
* We want to delete all the device files at regular cleanup time (after
unregistering with the hwmon class).
* It's OK to call device_create_file() on a non-existent file, so the
error path can be simplified.
I'd like the abituguru driver to behave the same as all other hardware
monitoring drivers to lower the maintenance effort. Can either you
or Jeff work up a compliant patch?
Thanks.
> drivers/hwmon/abituguru.c | 30 +++++++++++++++++++++++++-----
> 1 file changed, 25 insertions(+), 5 deletions(-)
>
> 2b10f648c8ed965369976eb7925b922ee187ce21
> diff --git a/drivers/hwmon/abituguru.c b/drivers/hwmon/abituguru.c
> index e5cb0fd..3ded982 100644
> --- a/drivers/hwmon/abituguru.c
> +++ b/drivers/hwmon/abituguru.c
> @@ -1271,14 +1271,34 @@ static int __devinit abituguru_probe(str
> res = PTR_ERR(data->class_dev);
> goto abituguru_probe_error;
> }
> - for (i = 0; i < sysfs_attr_i; i++)
> - device_create_file(&pdev->dev, &data->sysfs_attr[i].dev_attr);
> - for (i = 0; i < ARRAY_SIZE(abituguru_sysfs_attr); i++)
> - device_create_file(&pdev->dev,
> - &abituguru_sysfs_attr[i].dev_attr);
> + for (i = 0; i < sysfs_attr_i; i++) {
> + res = device_create_file(&pdev->dev,
> + &data->sysfs_attr[i].dev_attr);
> + if (res) {
> + for (j = 0; j < i; j++)
> + device_remove_file(&pdev->dev,
> + &data->sysfs_attr[j].dev_attr);
> + goto err_devreg;
> + }
> + }
> + for (i = 0; i < ARRAY_SIZE(abituguru_sysfs_attr); i++) {
> + res = device_create_file(&pdev->dev,
> + &abituguru_sysfs_attr[i].dev_attr);
> + if (res) {
> + for (j = 0; j < i; j++)
> + device_remove_file(&pdev->dev,
> + &abituguru_sysfs_attr[j].dev_attr);
> + goto err_attr_i;
> + }
> + }
>
> return 0;
>
> +err_attr_i:
> + for (i = 0; i < sysfs_attr_i; i++)
> + device_remove_file(&pdev->dev, &data->sysfs_attr[i].dev_attr);
> +err_devreg:
> + hwmon_device_unregister(data->class_dev);
> abituguru_probe_error:
> kfree(data);
> return res;
--
Jean Delvare
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
