| Message ID | d94376b6-12e8-45bb-a9be-4887bb316d35@web.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fpga: dfl: fme: Return directly after a failed devm_kasprintf() call in fme_perf_pmu_register() | expand |
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 > >
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 > > > > >
>>> 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
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
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
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
>> 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
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
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
>> 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
> 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
>> 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
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)