Re: [PATCH v2 4/4] platform/mellanox/mlxbf_pka: add __free(kfree) to handle memory allocation

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Mon, 11 Aug 2025, Ron Li wrote:

> Used __free(kfree) attribute to call kzalloc for allocating the 'data'

Hi Ron,

Grammar problems, but this patch won't be necessary, see below.

> variable in mlxbf_pka_drv_ring_ioctl(). So that the variable can be
> released automatically.
> 
> Reviewed-by: David Thompson <davthompson@nvidia.com>
> Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
> Signed-off-by: Ron Li <xiangrongl@nvidia.com>
> ---
>  drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c b/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c
> index 0f93c1aa7130..2dd773ec1dc3 100644
> --- a/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c
> +++ b/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c
> @@ -3,12 +3,14 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/cdev.h>
> +#include <linux/cleanup.h>
>  #include <linux/device.h>
>  #include <linux/hw_random.h>
>  #include <linux/idr.h>
>  #include <linux/interrupt.h>
>  #include <linux/iommu.h>
>  #include <linux/ioport.h>
> +#include <linux/kdev_t.h>
>  #include <linux/kernel.h>
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
> @@ -438,7 +440,6 @@ static long mlxbf_pka_drv_ring_ioctl(void *device_data, unsigned int cmd, unsign
>  		struct mlxbf_pka_dev_shim_s *shim;
>  		bool trng_present;
>  		u32 byte_cnt;
> -		u32 *data;
>  		int ret;
>  
>  		shim = ring_dev->ring->shim;
> @@ -456,25 +457,21 @@ static long mlxbf_pka_drv_ring_ioctl(void *device_data, unsigned int cmd, unsign
>  		 */
>  		byte_cnt = round_up(trng_data.count, MLXBF_PKA_TRNG_OUTPUT_CNT);
>  
> -		data = kzalloc(byte_cnt, GFP_KERNEL);
> +		u32 *data __free(kfree) = kzalloc(byte_cnt, GFP_KERNEL);
>  		if (!data)
>  			return -ENOMEM;
>  
>  		trng_present = mlxbf_pka_dev_has_trng(shim);
> -		if (!trng_present) {
> -			kfree(data);
> +		if (!trng_present)
>  			return -EAGAIN;
> -		}
>  
>  		ret = mlxbf_pka_dev_trng_read(shim, data, byte_cnt);
>  		if (ret) {
>  			dev_dbg(ring_dev->device, "TRNG failed %d\n", ret);
> -			kfree(data);
>  			return ret;
>  		}
>  
>  		ret = copy_to_user((void __user *)(trng_data.data), data, trng_data.count);
> -		kfree(data);
>  		return ret ? -EFAULT : 0;
>  	}

Please make the code to use the up-to-date conventions right from the 
start. We try to avoid changing same lines back and forth within the same 
series.

In general, anything newly introduced should use the most recent 
conventions. I might occassionally allow exceptions to this rule for 
a pre-existing file to remain consistent within that file but that
won't apply here.

-- 
 i.