Re: [PATCH] hwmon: (gpio-fan) Fix fan_ctrl_init error path

[Jean Delvare]

Hi Alex,

On Tue, 09 Nov 2010 13:18:39 +0800, Axel Lin wrote:
> Remove sysfs entries before return -ENODEV.
> 
> Signed-off-by: Axel Lin <axel.lin@xxxxxxxxx>
> ---
>  drivers/hwmon/gpio-fan.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> index aa701a1..d2e66b4 100644
> --- a/drivers/hwmon/gpio-fan.c
> +++ b/drivers/hwmon/gpio-fan.c
> @@ -388,11 +388,13 @@ static int fan_ctrl_init(struct gpio_fan_data *fan_data,
>  	fan_data->speed_index = get_fan_speed_index(fan_data);
>  	if (fan_data->speed_index < 0) {
>  		err = -ENODEV;
> -		goto err_free_gpio;
> +		goto err_remove_sysfs;
>  	}
>  
>  	return 0;
>  
> +err_remove_sysfs:
> +	sysfs_remove_group(&pdev->dev.kobj, &gpio_fan_ctrl_group);
>  err_free_gpio:
>  	for (i = i - 1; i >= 0; i--)
>  		gpio_free(ctrl[i]);

This is certainly better than the current situation, however the fact
that you need to do this strongly suggests that the initialization
order is wrong in the first place. Creating the sysfs attribute should
be the last thing done by the function, after all the rest has been
successful. Otherwise there is a small window during which user-space
can access the attribute but the driver isn't ready to deal with the
requests.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/