Message ID | 20241018140858.711-1-thunder.leizhen@huawei.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [1/1] coresight: trbe: Fix return value check in arm_trbe_register_coresight_cpu() | expand |
On 18/10/2024 3:08 pm, Zhen Lei wrote: > Function devm_kzalloc() returns NULL instead of ERR_PTR() when it fails. > The IS_ERR() test in the return value check should be replaced with NULL > test. > > Fixes: 39744738a67d ("coresight: trbe: Allocate platform data per device") > Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver") I don't think the code that this patches exists in 3fbf7f011f24, but it looks ok for 39744738a67d. > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > --- > drivers/hwtracing/coresight/coresight-trbe.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c > index 96a32b213669940..93fe9860acf16bd 100644 > --- a/drivers/hwtracing/coresight/coresight-trbe.c > +++ b/drivers/hwtracing/coresight/coresight-trbe.c > @@ -1266,7 +1266,7 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp > * into the device for that purpose. > */ > desc.pdata = devm_kzalloc(dev, sizeof(*desc.pdata), GFP_KERNEL); > - if (IS_ERR(desc.pdata)) > + if (!desc.pdata) > goto cpu_clear; > > desc.type = CORESIGHT_DEV_TYPE_SINK; Reviewed-by: James Clark <james.clark@linaro.org>
…
> The IS_ERR() test in the return value check should be replaced with NULL test.
…
How do you think about to convert such a change description into
an imperative wording?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.12-rc3#n94
Regards,
Markus
On 10/18/24 19:38, Zhen Lei wrote: > Function devm_kzalloc() returns NULL instead of ERR_PTR() when it fails. Right, devm_kzalloc() calls devm_kmalloc() with additional __GFP_ZERO which returns NULL on error. > The IS_ERR() test in the return value check should be replaced with NULL > test. > > Fixes: 39744738a67d ("coresight: trbe: Allocate platform data per device") > Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver") Actually this problem is caused by the following commit which had replaced coresight_get_platform_data() with devm_kzalloc() for a dummy 'desc.pdata' allocation. Earlier IS_ERR() test for the return value, was correct for coresight_get_platform_data() which returns ERR_PTR() on error, but then it should have been changed for devm_kzalloc() into a NULL check instead. 4277f035d227 ("coresight: trbe: Add a representative coresight_platform_data for TRBE") Please add "Fixes:" tag for the above commit instead. > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > --- > drivers/hwtracing/coresight/coresight-trbe.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c > index 96a32b213669940..93fe9860acf16bd 100644 > --- a/drivers/hwtracing/coresight/coresight-trbe.c > +++ b/drivers/hwtracing/coresight/coresight-trbe.c > @@ -1266,7 +1266,7 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp > * into the device for that purpose. > */ > desc.pdata = devm_kzalloc(dev, sizeof(*desc.pdata), GFP_KERNEL); > - if (IS_ERR(desc.pdata)) > + if (!desc.pdata) > goto cpu_clear; > > desc.type = CORESIGHT_DEV_TYPE_SINK; Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
On 2024/10/21 10:58, Anshuman Khandual wrote: > > > On 10/18/24 19:38, Zhen Lei wrote: >> Function devm_kzalloc() returns NULL instead of ERR_PTR() when it fails. > > Right, devm_kzalloc() calls devm_kmalloc() with additional __GFP_ZERO which > returns NULL on error. > >> The IS_ERR() test in the return value check should be replaced with NULL >> test. >> >> Fixes: 39744738a67d ("coresight: trbe: Allocate platform data per device") >> Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver") > > Actually this problem is caused by the following commit which had replaced > coresight_get_platform_data() with devm_kzalloc() for a dummy 'desc.pdata' > allocation. Earlier IS_ERR() test for the return value, was correct for > coresight_get_platform_data() which returns ERR_PTR() on error, but then > it should have been changed for devm_kzalloc() into a NULL check instead. > > 4277f035d227 ("coresight: trbe: Add a representative coresight_platform_data for TRBE") > > Please add "Fixes:" tag for the above commit instead. Yes, you're right. In retrospect, I probably found 3fbf7f011f24 just by searching for IS_ERR(). I will update and post v2. Thanks. > >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >> --- >> drivers/hwtracing/coresight/coresight-trbe.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c >> index 96a32b213669940..93fe9860acf16bd 100644 >> --- a/drivers/hwtracing/coresight/coresight-trbe.c >> +++ b/drivers/hwtracing/coresight/coresight-trbe.c >> @@ -1266,7 +1266,7 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp >> * into the device for that purpose. >> */ >> desc.pdata = devm_kzalloc(dev, sizeof(*desc.pdata), GFP_KERNEL); >> - if (IS_ERR(desc.pdata)) >> + if (!desc.pdata) >> goto cpu_clear; >> >> desc.type = CORESIGHT_DEV_TYPE_SINK; > > Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com> > > . >
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 96a32b213669940..93fe9860acf16bd 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1266,7 +1266,7 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp * into the device for that purpose. */ desc.pdata = devm_kzalloc(dev, sizeof(*desc.pdata), GFP_KERNEL); - if (IS_ERR(desc.pdata)) + if (!desc.pdata) goto cpu_clear; desc.type = CORESIGHT_DEV_TYPE_SINK;
Function devm_kzalloc() returns NULL instead of ERR_PTR() when it fails. The IS_ERR() test in the return value check should be replaced with NULL test. Fixes: 39744738a67d ("coresight: trbe: Allocate platform data per device") Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver") Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> --- drivers/hwtracing/coresight/coresight-trbe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)