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

[Maurizio Lombardi <mlombard@xxxxxxxxxx>]

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.