Subject: Re: [PATCH v2 1/2] apply: avoid fixed-size buffer in create_one_file()

From: Junio C Hamano <[email-masked]>
Date: Fri, 05 Apr 2024 09:51:23 -0700

René Scharfe <[email-masked]> writes:

> Changes since v1:
> - Split out removal of mksnpath() into a separate patch.
> - Use errno only before calling free(3).

Relative to the previous round, the body of the loop has become much
easier to follow.  Nice clean-up.

Thanks, both.

>  apply.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/apply.c b/apply.c
> index 432837a674..e171b42904 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -4441,6 +4441,7 @@ static int create_one_file(struct apply_state *state,
>  			   const char *buf,
>  			   unsigned long size)
>  {
> +	char *newpath = NULL;
>  	int res;
>
>  	if (state->cached)
> @@ -4502,24 +4503,26 @@ static int create_one_file(struct apply_state *state,
>  		unsigned int nr = getpid();
>
>  		for (;;) {
> -			char newpath[PATH_MAX];
> -			mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr);
> +			newpath = mkpathdup("%s~%u", path, nr);
>  			res = try_create_file(state, newpath, mode, buf, size);
>  			if (res < 0)
> -				return -1;
> +				goto out;
>  			if (!res) {
>  				if (!rename(newpath, path))
> -					return 0;
> +					goto out;
>  				unlink_or_warn(newpath);
>  				break;
>  			}
>  			if (errno != EEXIST)
>  				break;
>  			++nr;
> +			FREE_AND_NULL(newpath);
>  		}
>  	}
> -	return error_errno(_("unable to write file '%s' mode %o"),
> -			   path, mode);
> +	res = error_errno(_("unable to write file '%s' mode %o"), path, mode);
> +out:
> +	free(newpath);
> +	return res;
>  }
>
>  static int add_conflicted_stages_file(struct apply_state *state,
> --
> 2.44.0