[PATCH] media: platform: mtk-mdp3: Fix resource leaks in mdp_get_subsys_id()?

[Markus Elfring]

I would find an other patch subject more appropriate.


> Use put_device to release the object get through of_find_device_by_node,
> avoiding resource leaks.

How do you think about a wording variant for the change description
like the following?

* Add a put_device() call for the release of the object
  which was determined by a of_find_device_by_node() call.

* Use an additional label.

* Increase the usage of the variable “ret”.


> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
> @@ -892,11 +892,13 @@ static int mdp_get_subsys_id(struct mdp_dev *mdp, struct device *dev,
>  	ret = cmdq_dev_get_client_reg(&comp_pdev->dev, &cmdq_reg, index);
>  	if (ret != 0) {
>  		dev_err(&comp_pdev->dev, "cmdq_dev_get_subsys fail!\n");
> +		put_device(&comp_pdev->dev);
>  		return -EINVAL;
>  	}
>
>  	comp->subsys_id = cmdq_reg.subsys;
>  	dev_dbg(&comp_pdev->dev, "subsys id=%d\n", cmdq_reg.subsys);
> +	put_device(&comp_pdev->dev);
>
>  	return 0;
>  }

How will the chances evolve to integrate the following change variant?

 	ret = cmdq_dev_get_client_reg(&comp_pdev->dev, &cmdq_reg, index);
 	if (ret != 0) {
 		dev_err(&comp_pdev->dev, "cmdq_dev_get_subsys fail!\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto put_device;
 	}

 	comp->subsys_id = cmdq_reg.subsys;
 	dev_dbg(&comp_pdev->dev, "subsys id=%d\n", cmdq_reg.subsys);
+put_device:
+	put_device(&comp_pdev->dev);
-
-	return 0;
+	return ret;
 }


Regards,
Markus