linux-kernel - Re: [PATCH v2 2/8] iio: gts: Simplify using _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241123163713.2ec03a37@jic23-huawei>
Date: Sat, 23 Nov 2024 16:37:13 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>, Lars-Peter Clausen
 <lars@...afoo.de>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/8] iio: gts: Simplify using __free

On Thu, 21 Nov 2024 10:20:07 +0200
Matti Vaittinen <mazziesaccount@...il.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@...il.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@...il.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;
>  }
>  
>  /**