Re: [PATCH] enclosure: fix sysfs symlinks creation when using multipath

[Maurizio Lombardi]

Dne 16.3.2017 v 19:49 James Bottomley napsal(a): 
> diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
> index 65fed71..ae89082 100644
> --- a/drivers/misc/enclosure.c
> +++ b/drivers/misc/enclosure.c
> @@ -375,6 +375,7 @@ int enclosure_add_device(struct enclosure_device *edev, 
> int component,
>                        struct device *dev)
>  {
>       struct enclosure_component *cdev;
> +     int err;
>  
>       if (!edev || component >= edev->components)
>               return -EINVAL;
> @@ -384,12 +385,15 @@ int enclosure_add_device(struct enclosure_device *edev, 
> int component,
>       if (cdev->dev == dev)
>               return -EEXIST;
>  
> -     if (cdev->dev)
> +     if (cdev->dev) {
>               enclosure_remove_links(cdev);
> -
> -     put_device(cdev->dev);
> -     cdev->dev = get_device(dev);
> -     return enclosure_add_links(cdev);
> +             put_device(cdev->dev);
> +             cdev->dev = NULL;
> +     }
> +     err = enclosure_add_links(cdev);
> +     if (!err)
> +             cdev->dev = get_device(dev);
> +     return err;
>  }
>  EXPORT_SYMBOL_GPL(enclosure_add_device);
>  
>


I will ask our customer to test your patch,
there is only a small problem: you can't set cdev->dev = NULL
and then call enclosure_add_links(cdev) because you will end up dereferencing
a NULL pointer.
I suggest a slightly different patch:

@@ -375,6 +379,7 @@ int enclosure_add_device(struct enclosure_device *edev, int 
component,
                         struct device *dev)
 {
        struct enclosure_component *cdev;
+       int err;
 
        if (!edev || component >= edev->components)
                return -EINVAL;
@@ -384,12 +389,18 @@ int enclosure_add_device(struct enclosure_device *edev, 
int component,
        if (cdev->dev == dev)
                return -EEXIST;
 
-       if (cdev->dev)
+       if (cdev->dev) {
                enclosure_remove_links(cdev);
-
-       put_device(cdev->dev);
+               put_device(cdev->dev);
+       }
        cdev->dev = get_device(dev);
-       return enclosure_add_links(cdev);
+
+       err = enclosure_add_links(cdev);
+       if (err) {
+               put_device(cdev->dev);
+               cdev->dev = NULL;
+       }
+       return err;
 }
 EXPORT_SYMBOL_GPL(enclosure_add_device);
 
Thanks,
Maurizio.