Re: [PATCH v2 2/8] iio: gts: Simplify using __free

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 21 Nov 2024 10:20:07 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> The error path in the gain_to_scaletables() uses goto for unwinding an
> allocation on failure. This can be slightly simplified by using the
> automated free when exiting the scope.
> 
> Use __free(kfree) and drop the goto based error handling.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---
> 
> Revision history:
>   v1 => v2:
>   - patch number changed because a change was added to the series.
>   - rebased on iio/testing to avoid conflicts with queued fixes
> ---
>  drivers/iio/industrialio-gts-helper.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c
> index 291c0fc332c9..602d3d338e66 100644
> --- a/drivers/iio/industrialio-gts-helper.c
> +++ b/drivers/iio/industrialio-gts-helper.c
> @@ -4,6 +4,7 @@
>   * Copyright (c) 2023 Matti Vaittinen <mazziesaccount@gmail.com>
>   */
>  
> +#include <linux/cleanup.h>
>  #include <linux/device.h>
>  #include <linux/errno.h>
>  #include <linux/export.h>
> @@ -167,8 +168,8 @@ static int iio_gts_gain_cmp(const void *a, const void *b)
>  
>  static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales)
>  {
> -	int i, j, new_idx, time_idx, ret = 0;
> -	int *all_gains;
> +	int ret, i, j, new_idx, time_idx;
> +	int *all_gains __free(kfree) = NULL;
See the docs in cleanup.h (added recently).

Constructor and destructor should go together.   Dan wrote good docs on this
(which are now in cleanup.h) so I'll not go into why!

Upshot is this goes where you do the kcalloc, not up here.

>  	size_t gain_bytes;
>  
>  	for (i = 0; i < gts->num_itime; i++) {
> @@ -232,10 +233,9 @@ static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales)
>  
>  	gts->avail_all_scales_table = kcalloc(new_idx, 2 * sizeof(int),
>  					      GFP_KERNEL);
> -	if (!gts->avail_all_scales_table) {
> -		ret = -ENOMEM;
> -		goto free_out;
> -	}
> +	if (!gts->avail_all_scales_table)
> +		return -ENOMEM;
> +
>  	gts->num_avail_all_scales = new_idx;
>  
>  	for (i = 0; i < gts->num_avail_all_scales; i++) {
> @@ -246,14 +246,11 @@ static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales)
>  		if (ret) {
>  			kfree(gts->avail_all_scales_table);
>  			gts->num_avail_all_scales = 0;
> -			goto free_out;
> +			return ret;
>  		}
>  	}
>  
> -free_out:
> -	kfree(all_gains);
> -
> -	return ret;
> +	return 0;
>  }
>  
>  /**