Re: [PATCH v1] leds: Allow to call led_classdev_unregister() unconditionally

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

Hi!

> If in the certain driver the LED is optional, and it's a majority of them,
> the call of led_classdev_unregister() still requires some additional checks.
> 
> The usual pattern on unregistering is to check for NULL, but we also check
> for IS_ERR() in case device_create_with_groups() fails.
> 
> The change will reduce a burden in a lot of drivers to repeatedly check
> for above conditions.

I don't see majority of calls being protected.  Doing nothing on NULL
sounds reasonable. I'm less sure about "IS_ERR"...

Best regards,
								Pavel


> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/leds/led-class.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index d231240c2047..80b62b807ea0 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -287,6 +287,7 @@ int led_classdev_register_ext(struct device *parent,
>  		ret = led_add_brightness_hw_changed(led_cdev);
>  		if (ret) {
>  			device_unregister(led_cdev->dev);
> +			led_cdev->dev = NULL;
>  			mutex_unlock(&led_cdev->led_access);
>  			return ret;
>  		}
> @@ -332,6 +333,9 @@ EXPORT_SYMBOL_GPL(led_classdev_register_ext);
>   */
>  void led_classdev_unregister(struct led_classdev *led_cdev)
>  {
> +	if (IS_ERR_OR_NULL(led_cdev->dev))
> +		return;
> +
>  #ifdef CONFIG_LEDS_TRIGGERS
>  	down_write(&led_cdev->trigger_lock);
>  	if (led_cdev->trigger)

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]