Re: [PATCH] tools/iio/iio_utils:fix memory leak

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 12 Jan 2023 19:01:22 +0800
Yulong Zhang <yulong.zhang@metoak.net> wrote:

> 1.fopen sysfsfp without fclose
> 2.asprintf filename without free
> 
> Signed-off-by: Yulong Zhang <yulong.zhang@metoak.net>
Good spot.  One suggestion below on how to tidy up the code
whilst fixing this.

> ---
>  tools/iio/iio_utils.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/iio/iio_utils.c b/tools/iio/iio_utils.c
> index 8d35893b2..38e9352e5 100644
> --- a/tools/iio/iio_utils.c
> +++ b/tools/iio/iio_utils.c
> @@ -264,6 +264,8 @@ int iioutils_get_param_float(float *output, const char *param_name,
>  			if (fscanf(sysfsfp, "%f", output) != 1)
>  				ret = errno ? -errno : -ENODATA;
>  
> +			fclose(sysfsfp);
> +
>  			break;
>  		}
>  error_free_filename:
> @@ -444,6 +446,7 @@ int build_channel_array(const char *device_dir, int buffer_idx,
>  				count--;
>  				goto error_cleanup_array;
>  			}
> +			free(filename);

Instead of doing it here, why not move the free as early as possible (and hence
drop the need to do it in most of the error paths.
			sysfsfp = fopen(filename, "r");
		+	free(filename);
			if (!sysfsfp) {
				ret = -errno;
		-		free(filename)
... and the other cases that follow.

The same could be done for the case below, at the minor cost of not
having the filename for the print statement (which I think would be fine).

>  
>  			ret = asprintf(&filename,
>  				       "%s/%s_index",