[PATCH i-g-t v4 2/4] lib/igt_sysfs: stop leaking fd on write failures

[Janusz Krzysztofik]

Hi Lucas,

On Wednesday, 28 February 2024 23:31:32 CET Lucas De Marchi wrote:
> Make sure to close the fd before returning.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  lib/igt_sysfs.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> index 2b0225138..2997925e5 100644
> --- a/lib/igt_sysfs.c
> +++ b/lib/igt_sysfs.c
> @@ -515,29 +515,36 @@ int igt_sysfs_vprintf(int dir, const char *attr, const char *fmt, va_list ap)
>  	va_copy(tmp, ap);
>  	ret = vsnprintf(stack, sizeof(stack), fmt, tmp);
>  	va_end(tmp);
> -	if (igt_debug_on(ret < 0))
> -		return -EINVAL;
> +	if (igt_debug_on(ret < 0)) {
> +		ret = -EINVAL;
> +		goto end;
> +	}
>  
>  	if (ret > sizeof(stack)) {
>  		unsigned int len = ret + 1;
>  
>  		buf = malloc(len);
> -		if (igt_debug_on(!buf))
> -			return -ENOMEM;
> +		if (igt_debug_on(!buf)) {
> +			ret = -ENOMEM;
> +			goto end;
> +		}
>  
>  		ret = vsnprintf(buf, ret, fmt, ap);
>  		if (igt_debug_on(ret > len)) {
> -			free(buf);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto free_buf;
>  		}
>  	}
>  
>  	ret = igt_writen(fd, buf, ret);
>  
> -	close(fd);

How about moving the code that opens the fd from the top of the body of this 
function to just in front of the call to igt_writen(), and keeping this 
close() here?  That way we wouldn't need the end: section of the error unwind 
path, nor those two jumps to it.

Thanks,
Janusz


> +free_buf:
>  	if (buf != stack)
>  		free(buf);
>  
> +end:
> +	close(fd);
> +
>  	return ret;
>  }
>  
>