[PATCH] fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[PATCH] fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 27 Jan 2024 15:43:42 +0100

The result from a call of the function “devm_kasprintf” was passed to
a subsequent function call without checking for a null pointer before
(according to a memory allocation failure).
This issue was detected by using the Coccinelle software.

Thus return directly after a failed devm_kasprintf() call.

Fixes: 724142f8c42a7 ("fpga: dfl: fme: add performance reporting support")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/fpga/dfl-fme-perf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/fpga/dfl-fme-perf.c b/drivers/fpga/dfl-fme-perf.c
index 7422d2bc6f37..db56d52411ef 100644
--- a/drivers/fpga/dfl-fme-perf.c
+++ b/drivers/fpga/dfl-fme-perf.c
@@ -925,6 +925,8 @@ static int fme_perf_pmu_register(struct platform_device *pdev,
 				PERF_PMU_CAP_NO_EXCLUDE;

 	name = devm_kasprintf(priv->dev, GFP_KERNEL, "dfl_fme%d", pdev->id);
+	if (!name)
+		return -ENOMEM;

 	ret = perf_pmu_register(pmu, name, -1);
 	if (ret)
--
2.43.0

Re: [PATCH] fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Xu Yilun]

On Sat, Jan 27, 2024 at 03:55:19PM +0100, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 27 Jan 2024 15:43:42 +0100
> 
> The result from a call of the function “devm_kasprintf” was passed to
> a subsequent function call without checking for a null pointer before
> (according to a memory allocation failure).
> This issue was detected by using the Coccinelle software.
> 
> Thus return directly after a failed devm_kasprintf() call.
> 
> Fixes: 724142f8c42a7 ("fpga: dfl: fme: add performance reporting support")
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Xu Yilun <yilun.xu@intel.com>

> ---
>  drivers/fpga/dfl-fme-perf.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/fpga/dfl-fme-perf.c b/drivers/fpga/dfl-fme-perf.c
> index 7422d2bc6f37..db56d52411ef 100644
> --- a/drivers/fpga/dfl-fme-perf.c
> +++ b/drivers/fpga/dfl-fme-perf.c
> @@ -925,6 +925,8 @@ static int fme_perf_pmu_register(struct platform_device *pdev,
>  				PERF_PMU_CAP_NO_EXCLUDE;
> 
>  	name = devm_kasprintf(priv->dev, GFP_KERNEL, "dfl_fme%d", pdev->id);
> +	if (!name)
> +		return -ENOMEM;
> 
>  	ret = perf_pmu_register(pmu, name, -1);
>  	if (ret)
> --
> 2.43.0
> 
> 

Re: [PATCH] fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Xu Yilun]

On Tue, Jan 30, 2024 at 06:03:12PM +0800, Xu Yilun wrote:
> On Sat, Jan 27, 2024 at 03:55:19PM +0100, Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sat, 27 Jan 2024 15:43:42 +0100

Sorry, something to fix.

Please shorten your shortlog to less than 75 chars.
Please refer to Documentation/process/submitting-patches.rst

> > 
> > The result from a call of the function “devm_kasprintf” was passed to
> > a subsequent function call without checking for a null pointer before
> > (according to a memory allocation failure).
> > This issue was detected by using the Coccinelle software.
> > 
> > Thus return directly after a failed devm_kasprintf() call.
> > 
> > Fixes: 724142f8c42a7 ("fpga: dfl: fme: add performance reporting support")

One more char of sha.

Please use checkpatch to verify.

Thanks,
Yilun

> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> 
> Acked-by: Xu Yilun <yilun.xu@intel.com>
> 
> > ---
> >  drivers/fpga/dfl-fme-perf.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/fpga/dfl-fme-perf.c b/drivers/fpga/dfl-fme-perf.c
> > index 7422d2bc6f37..db56d52411ef 100644
> > --- a/drivers/fpga/dfl-fme-perf.c
> > +++ b/drivers/fpga/dfl-fme-perf.c
> > @@ -925,6 +925,8 @@ static int fme_perf_pmu_register(struct platform_device *pdev,
> >  				PERF_PMU_CAP_NO_EXCLUDE;
> > 
> >  	name = devm_kasprintf(priv->dev, GFP_KERNEL, "dfl_fme%d", pdev->id);
> > +	if (!name)
> > +		return -ENOMEM;
> > 
> >  	ret = perf_pmu_register(pmu, name, -1);
> >  	if (ret)
> > --
> > 2.43.0
> > 
> > 
> 

Re: fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Markus Elfring]

>>> Thus return directly after a failed devm_kasprintf() call.
>>>
>>> Fixes: 724142f8c42a7 ("fpga: dfl: fme: add performance reporting support")
>
> One more char of sha.

There are different preferences involved.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.8-rc2#n109

Regards,
Markus

Re: fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Xu Yilun]

On Tue, Jan 30, 2024 at 11:48:31AM +0100, Markus Elfring wrote:
> >>> Thus return directly after a failed devm_kasprintf() call.
> >>>
> >>> Fixes: 724142f8c42a7 ("fpga: dfl: fme: add performance reporting support")
> >
> > One more char of sha.
> 
> There are different preferences involved.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.8-rc2#n109

Ah, I mean you use 13 chars, but 12 chars is better. Also the doc
doens't seem to enforce 12 chars, but checkpatch warns on that. So just
follow the 12 chars style.

Thanks,
Yilun

> 
> Regards,
> Markus

Re: [PATCH] fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Greg Kroah-Hartman]

On Tue, Jan 30, 2024 at 06:27:25PM +0800, Xu Yilun wrote:
> On Tue, Jan 30, 2024 at 06:03:12PM +0800, Xu Yilun wrote:
> > On Sat, Jan 27, 2024 at 03:55:19PM +0100, Markus Elfring wrote:
> > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > Date: Sat, 27 Jan 2024 15:43:42 +0100
> 
> Sorry, something to fix.
> 
> Please shorten your shortlog to less than 75 chars.
> Please refer to Documentation/process/submitting-patches.rst
> 
> > > 
> > > The result from a call of the function “devm_kasprintf” was passed to
> > > a subsequent function call without checking for a null pointer before
> > > (according to a memory allocation failure).
> > > This issue was detected by using the Coccinelle software.
> > > 
> > > Thus return directly after a failed devm_kasprintf() call.
> > > 
> > > Fixes: 724142f8c42a7 ("fpga: dfl: fme: add performance reporting support")
> 
> One more char of sha.
> 
> Please use checkpatch to verify.
> 
> Thanks,
> Yilun

Hi,

This is the semi-friendly patch-bot of Greg Kroah-Hartman.

Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list.  I strongly suggest that you not do this anymore.  Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.

Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all.  The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback.  Please feel free to also ignore emails
from them.

thanks,

greg k-h's patch email bot

Re: [PATCH] fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Dan Carpenter]

On Sat, Jan 27, 2024 at 03:55:19PM +0100, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 27 Jan 2024 15:43:42 +0100
> 
> The result from a call of the function “devm_kasprintf” was passed to
> a subsequent function call without checking for a null pointer before
> (according to a memory allocation failure).
> This issue was detected by using the Coccinelle software.
> 
> Thus return directly after a failed devm_kasprintf() call.
> 
> Fixes: 724142f8c42a7 ("fpga: dfl: fme: add performance reporting support")

This basically doesn't affect runtime because perf_pmu_register() checks
for NULL so no need for a Fixes tag.

regards,
dan carpenter

Re: fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Markus Elfring]

>> Thus return directly after a failed devm_kasprintf() call.
>>
>> Fixes: 724142f8c42a7 ("fpga: dfl: fme: add performance reporting support")
>
> This basically doesn't affect runtime because perf_pmu_register() checks
> for NULL so no need for a Fixes tag.

I suggest to clarify this view a bit more also according to statements
like the following.

1. https://elixir.bootlin.com/linux/v6.8-rc2/source/kernel/events/core.c#L11532
   perf_pmu_register:
   …
	pmu->name = name;
   …

2. https://elixir.bootlin.com/linux/v6.8-rc2/source/kernel/events/core.c#L11472
   pmu_dev_alloc:
   …
	ret = dev_set_name(pmu->dev, "%s", pmu->name);
   …


Regards,
Markus

Re: fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Greg Kroah-Hartman]

On Tue, Jan 30, 2024 at 06:09:14PM +0100, Markus Elfring wrote:
> >> Thus return directly after a failed devm_kasprintf() call.
> >>
> >> Fixes: 724142f8c42a7 ("fpga: dfl: fme: add performance reporting support")
> >
> > This basically doesn't affect runtime because perf_pmu_register() checks
> > for NULL so no need for a Fixes tag.
> 
> I suggest to clarify this view a bit more also according to statements
> like the following.
> 
> 1. https://elixir.bootlin.com/linux/v6.8-rc2/source/kernel/events/core.c#L11532
>    perf_pmu_register:
>    …
> 	pmu->name = name;
>    …
> 
> 2. https://elixir.bootlin.com/linux/v6.8-rc2/source/kernel/events/core.c#L11472
>    pmu_dev_alloc:
>    …
> 	ret = dev_set_name(pmu->dev, "%s", pmu->name);
>    …
> 
> 
> Regards,
> Markus

Hi,

This is the semi-friendly patch-bot of Greg Kroah-Hartman.

Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list.  I strongly suggest that you not do this anymore.  Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.

thanks,

greg k-h's patch email bot

Re: fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Dan Carpenter]

On Tue, Jan 30, 2024 at 06:09:14PM +0100, Markus Elfring wrote:
> >> Thus return directly after a failed devm_kasprintf() call.
> >>
> >> Fixes: 724142f8c42a7 ("fpga: dfl: fme: add performance reporting support")
> >
> > This basically doesn't affect runtime because perf_pmu_register() checks
> > for NULL so no need for a Fixes tag.
> 
> I suggest to clarify this view a bit more also according to statements
> like the following.
> 
> 1. https://elixir.bootlin.com/linux/v6.8-rc2/source/kernel/events/core.c#L11532
>    perf_pmu_register:
>    …
> 	pmu->name = name;
>    …

The check is right before that on line 11527.

https://elixir.bootlin.com/linux/v6.8-rc2/source/kernel/events/core.c#L11527

regards,
dan carpenter

Re: fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Markus Elfring]

>> There are different preferences involved.
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.8-rc2#n109
>
> Ah, I mean you use 13 chars, but 12 chars is better. Also the doc
> doens't seem to enforce 12 chars, but checkpatch warns on that.

Would the specification “at least” become supported at further places
for such hash lengths?

Regards,
Markus

Re: fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Markus Elfring]

> The check is right before that on line 11527.
>
> https://elixir.bootlin.com/linux/v6.8-rc2/source/kernel/events/core.c#L11527

Do you find a warning message like “Can not register anonymous pmu.” acceptable
in addition to eventual other indications for memory allocation failures so far?

Regards,
Markus

Re: fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register()

[Markus Elfring]

>> Thus return directly after a failed devm_kasprintf() call.
>>
>> Fixes: 724142f8c42a7 ("fpga: dfl: fme: add performance reporting support")
>
> This basically doesn't affect runtime because perf_pmu_register() checks for NULL

https://elixir.bootlin.com/linux/v6.8-rc2/source/kernel/events/core.c#L11527

Do you prefer the usage of the error code “-ENOMEM” instead of “-EINVAL”
as an indication for a failed memory allocation?


> so no need for a Fixes tag.

Does the selection of a more appropriate error code qualify for this tag?

Regards,
Markus