This validates 'drvdata' and 'drvdata->pclk' clock before calling clk_put() in etm4_remove_platform_dev(). The problem was detected using Smatch static checker as reported.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: James Clark james.clark@arm.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lists.linaro.org/archives/list/coresight@lists.linaro.org/thread/G4N... Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- This applies on coresight-next
drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 703b6fcbb6a5..eb412ce302cc 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2269,7 +2269,7 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) etm4_remove_dev(drvdata); pm_runtime_disable(&pdev->dev);
- if (drvdata->pclk) + if (drvdata && drvdata->pclk && !IS_ERR(drvdata->pclk)) clk_put(drvdata->pclk);
return 0;
On 11/08/2023 07:27, Anshuman Khandual wrote:
This validates 'drvdata' and 'drvdata->pclk' clock before calling clk_put() in etm4_remove_platform_dev(). The problem was detected using Smatch static checker as reported.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: James Clark james.clark@arm.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lists.linaro.org/archives/list/coresight@lists.linaro.org/thread/G4N... Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
This applies on coresight-next
drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 703b6fcbb6a5..eb412ce302cc 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2269,7 +2269,7 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) etm4_remove_dev(drvdata); pm_runtime_disable(&pdev->dev);
- if (drvdata->pclk)
- if (drvdata && drvdata->pclk && !IS_ERR(drvdata->pclk)) clk_put(drvdata->pclk);
return 0;
It could be !IS_ERR_OR_NULL(drvdata->pclk), but I wouldn't bother changing it at this point.
Reviewed-by: James Clark james.clark@arm.com
On 11/08/2023 09:39, James Clark wrote:
On 11/08/2023 07:27, Anshuman Khandual wrote:
This validates 'drvdata' and 'drvdata->pclk' clock before calling clk_put() in etm4_remove_platform_dev(). The problem was detected using Smatch static checker as reported.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: James Clark james.clark@arm.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lists.linaro.org/archives/list/coresight@lists.linaro.org/thread/G4N... Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
This applies on coresight-next
drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 703b6fcbb6a5..eb412ce302cc 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2269,7 +2269,7 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) etm4_remove_dev(drvdata); pm_runtime_disable(&pdev->dev);
- if (drvdata->pclk)
- if (drvdata && drvdata->pclk && !IS_ERR(drvdata->pclk)) clk_put(drvdata->pclk);
return 0;
It could be !IS_ERR_OR_NULL(drvdata->pclk), but I wouldn't bother changing it at this point.
+1, please could we have that. Someone else will run a code scanner and send a patch later. Given this is straight and easy change, lets do it in the first place.
Cheers Suzuki
Reviewed-by: James Clark james.clark@arm.com
On 8/11/23 14:39, Suzuki K Poulose wrote:
On 11/08/2023 09:39, James Clark wrote:
On 11/08/2023 07:27, Anshuman Khandual wrote:
This validates 'drvdata' and 'drvdata->pclk' clock before calling clk_put() in etm4_remove_platform_dev(). The problem was detected using Smatch static checker as reported.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: James Clark james.clark@arm.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lists.linaro.org/archives/list/coresight@lists.linaro.org/thread/G4N... Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
This applies on coresight-next
drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 703b6fcbb6a5..eb412ce302cc 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2269,7 +2269,7 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) etm4_remove_dev(drvdata); pm_runtime_disable(&pdev->dev); - if (drvdata->pclk) + if (drvdata && drvdata->pclk && !IS_ERR(drvdata->pclk)) clk_put(drvdata->pclk); return 0;
It could be !IS_ERR_OR_NULL(drvdata->pclk), but I wouldn't bother changing it at this point.
+1, please could we have that. Someone else will run a code scanner and send a patch later. Given this is straight and easy change, lets do it in the first place.
But we already have a drvdata->pclk validation check before IS_ERR(). Would not _OR_NULL be redundant ?
On 11/08/2023 10:22, Anshuman Khandual wrote:
On 8/11/23 14:39, Suzuki K Poulose wrote:
On 11/08/2023 09:39, James Clark wrote:
On 11/08/2023 07:27, Anshuman Khandual wrote:
This validates 'drvdata' and 'drvdata->pclk' clock before calling clk_put() in etm4_remove_platform_dev(). The problem was detected using Smatch static checker as reported.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: James Clark james.clark@arm.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lists.linaro.org/archives/list/coresight@lists.linaro.org/thread/G4N... Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
This applies on coresight-next
drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 703b6fcbb6a5..eb412ce302cc 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2269,7 +2269,7 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) etm4_remove_dev(drvdata); pm_runtime_disable(&pdev->dev); - if (drvdata->pclk) + if (drvdata && drvdata->pclk && !IS_ERR(drvdata->pclk)) clk_put(drvdata->pclk); return 0;
It could be !IS_ERR_OR_NULL(drvdata->pclk), but I wouldn't bother changing it at this point.
+1, please could we have that. Someone else will run a code scanner and send a patch later. Given this is straight and easy change, lets do it in the first place.
But we already have a drvdata->pclk validation check before IS_ERR(). Would not _OR_NULL be redundant ?
I meant that it could be replaced with the single check:
if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) clk_put(drvdata->pclk);
As Dan mentions it can't be an error pointer anyway, but leaving it like this could just be considered defensive coding.
On 8/11/23 15:44, James Clark wrote:
On 11/08/2023 10:22, Anshuman Khandual wrote:
On 8/11/23 14:39, Suzuki K Poulose wrote:
On 11/08/2023 09:39, James Clark wrote:
On 11/08/2023 07:27, Anshuman Khandual wrote:
This validates 'drvdata' and 'drvdata->pclk' clock before calling clk_put() in etm4_remove_platform_dev(). The problem was detected using Smatch static checker as reported.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: James Clark james.clark@arm.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lists.linaro.org/archives/list/coresight@lists.linaro.org/thread/G4N... Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
This applies on coresight-next
drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 703b6fcbb6a5..eb412ce302cc 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2269,7 +2269,7 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) etm4_remove_dev(drvdata); pm_runtime_disable(&pdev->dev); - if (drvdata->pclk) + if (drvdata && drvdata->pclk && !IS_ERR(drvdata->pclk)) clk_put(drvdata->pclk); return 0;
It could be !IS_ERR_OR_NULL(drvdata->pclk), but I wouldn't bother changing it at this point.
+1, please could we have that. Someone else will run a code scanner and send a patch later. Given this is straight and easy change, lets do it in the first place.
But we already have a drvdata->pclk validation check before IS_ERR(). Would not _OR_NULL be redundant ?
I meant that it could be replaced with the single check:
if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) clk_put(drvdata->pclk);
As Dan mentions it can't be an error pointer anyway, but leaving it like this could just be considered defensive coding.
Let's just go with the above change as you had suggested unless there is any particular objection.
if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) clk_put(drvdata->pclk);
On Fri, 11 Aug 2023 at 07:27, Anshuman Khandual anshuman.khandual@arm.com wrote:
This validates 'drvdata' and 'drvdata->pclk' clock before calling clk_put() in etm4_remove_platform_dev(). The problem was detected using Smatch static checker as reported.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: James Clark james.clark@arm.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lists.linaro.org/archives/list/coresight@lists.linaro.org/thread/G4N... Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
This applies on coresight-next
drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 703b6fcbb6a5..eb412ce302cc 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2269,7 +2269,7 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) etm4_remove_dev(drvdata); pm_runtime_disable(&pdev->dev);
if (drvdata->pclk)
if (drvdata && drvdata->pclk && !IS_ERR(drvdata->pclk)) clk_put(drvdata->pclk); return 0;-- 2.25.1
Reviewed-by: Mike Leach mike.leach@lnaro.org
-
Anshuman Khandual -
James Clark -
Mike Leach -
Suzuki K Poulose