Re: [PATCH] mailbox: pcc: Add missed acpi_put_table() to fix memory leak

Re: [PATCH] mailbox: pcc: Add missed acpi_put_table() to fix memory leak

[Markus Elfring]

> In pcc_mbox_probe(), the PCCT table acquired via acpi_get_table()
> is only released in error paths but not in the success path. This
> leads to a permanent ACPI memory leak when the driver successfully
> initializes.

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16#n145


> The label name 'err' is no longer accurate because it handles both:
> 1. Error cases
> 2. Success case

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16#n94


…
> +++ b/drivers/mailbox/pcc.c
> @@ -763,19 +763,19 @@ static int pcc_mbox_probe(struct platform_device *pdev)
>  					 GFP_KERNEL);
>  	if (!pcc_mbox_channels) {
>  		rc = -ENOMEM;
> -		goto err;
> +		goto out_put_pcct;
>  	}
>  
>  	chan_info = devm_kcalloc(dev, count, sizeof(*chan_info), GFP_KERNEL);
>  	if (!chan_info) {
>  		rc = -ENOMEM;
> -		goto err;
> +		goto out_put_pcct;
>  	}
>  
>  	pcc_mbox_ctrl = devm_kzalloc(dev, sizeof(*pcc_mbox_ctrl), GFP_KERNEL);
>  	if (!pcc_mbox_ctrl) {
>  		rc = -ENOMEM;
> -		goto err;
> +		goto out_put_pcct;
>  	}
…

Can such exception handling be shared by using another jump target like “e_nomem”?

Regards,
Markus

Re: [PATCH v2] mailbox: pcc: Add missed acpi_put_table() to fix memory leak

[Markus Elfring]

> In pcc_mbox_probe(), the PCCT table acquired via acpi_get_table()
> is only released in error paths but not in the success path. This
> leads to a permanent ACPI memory leak when the driver successfully
> initializes.

* You may occasionally put more than 66 characters into text lines
  of such a change description.
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16#n658

* See also once more:
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16#n94


…> ---
> Changes in v2:
> - Add tags of 'Fixes' and 'Cc'
> - Change goto target from out_put_pcct to e_nomem
…> +++ b/drivers/mailbox/pcc.c
> @@ -763,19 +763,19 @@ static int pcc_mbox_probe(struct platform_device *pdev)
>  					 GFP_KERNEL);
>  	if (!pcc_mbox_channels) {
>  		rc = -ENOMEM;
> -		goto err;
> +		goto e_nomem;
>  	}
…

Why do you not move the assignment statement accordingly?


…> @@ -796,17 +796,17 @@ static int pcc_mbox_probe(struct platform_device *pdev)
>  		    !pcc_mbox_ctrl->txdone_irq) {
>  			pr_err("Platform Interrupt flag must be set to 1");
>  			rc = -EINVAL;
> -			goto err;
> +			goto e_nomem;
>  		}
…

You misunderstood one of my development suggestions.


…> @@ -827,9 +827,8 @@ static int pcc_mbox_probe(struct platform_device *pdev)
…> -		return 0;
> -err:
> +
> +e_nomem:
>  	acpi_put_table(pcct_tbl);
>  	return rc;
>  }
Would an other code variant be more reasonable?

e_nomem:
	rc = -ENOMEM;
	goto err;


Regards,
Markus

Re: [PATCH v3] mailbox: pcc: Add missed acpi_put_table() to fix memory leak

[Markus Elfring]

> In pcc_mbox_probe(), the PCCT table acquired via acpi_get_table() is
> only released in error paths but not in the success path. Fix a
> permanent ACPI memory leak when the driver successfully initializes. Add
> the goto label 'err_nomem'.

You may use an enumeration for your change description.

* Fix …

* Add … so that a bit of exception handling can be refined.


…> +++ b/drivers/mailbox/pcc.c
…> @@ -827,8 +821,11 @@ static int pcc_mbox_probe(struct platform_device *pdev)
…> +err_nomem:
> +	rc = -ENOMEM;
> +	goto err;

Please omit such a redundant statement at this proposed place.


>  err:
>  	acpi_put_table(pcct_tbl);
>  	return rc;
Would a label like “put_table” become helpful here?

Regards,
Markus

Re: [PATCH v4] mailbox: pcc: Add missed acpi_put_table() to fix memory leak

[Markus Elfring]

> Fixes a permanent ACPI memory leak in the success path by adding
> acpi_put_table().
> Renaming generic 'err' label to 'put_table' for clarity.

Will a desire grow for the usage of imperative mood also in such change descriptions?

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16#n94


…> Changes in v4:
> - Change goto target from err to put_table.

Thanks.


> - Remove goto tatget err_nomem
…

Does this adjustment indicate questionable development difficulties?

Regards,
Markus

Re: [PATCH v4] mailbox: pcc: Add missed acpi_put_table() to fix memory leak

[Greg KH]

On Tue, Aug 05, 2025 at 08:28:17AM +0200, Markus Elfring wrote:
> > Fixes a permanent ACPI memory leak in the success path by adding
> > acpi_put_table().
> > Renaming generic 'err' label to 'put_table' for clarity.
> 
> Will a desire grow for the usage of imperative mood also in such change descriptions?
> 
> See also once more:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16#n94
> 
> 
> …> Changes in v4:
> > - Change goto target from err to put_table.
> 
> Thanks.
> 
> 
> > - Remove goto tatget err_nomem
> …
> 
> Does this adjustment indicate questionable development difficulties?
> 
> 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.

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