Re: [PATCH 08/14] tree-diff: pass whole path string to path_appendnew()

[Patrick Steinhardt <ps@pks.im>]

On Thu, Jan 09, 2025 at 03:49:07AM -0500, Jeff King wrote:
> diff --git a/tree-diff.c b/tree-diff.c
> index 22fc2d8f8c..d2f8dd14a6 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -129,20 +129,18 @@ static int emit_diff_first_parent_only(struct diff_options *opt, struct combine_
>   * and append it to paths list tail.
>   */
>  static struct combine_diff_path *path_appendnew(struct combine_diff_path *last,
> -	int nparent, const struct strbuf *base, const char *path, int pathlen,
> +	int nparent, const char *path, size_t len,

Sneaky, you also changed the type of `len` :) You might want to point
that out in the commit message.

>  	unsigned mode, const struct object_id *oid)
>  {
>  	struct combine_diff_path *p;
> -	size_t len = st_add(base->len, pathlen);
>  	size_t alloclen = combine_diff_path_size(nparent, len);
>  
>  	p = xmalloc(alloclen);
>  	p->next = NULL;
>  	last->next = p;
>  
>  	p->path = (char *)&(p->parent[nparent]);
> -	memcpy(p->path, base->buf, base->len);
> -	memcpy(p->path + base->len, path, pathlen);
> +	memcpy(p->path, path, len);
>  	p->path[len] = 0;
>  	p->mode = mode;
>  	oidcpy(&p->oid, oid ? oid : null_oid());
> @@ -206,7 +204,10 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
>  	if (emitthis) {
>  		int keep;
>  		struct combine_diff_path *pprev = p;
> -		p = path_appendnew(p, nparent, base, path, pathlen, mode, oid);
> +
> +		strbuf_add(base, path, pathlen);
> +		p = path_appendnew(p, nparent, base->buf, base->len, mode, oid);
> +		strbuf_setlen(base, old_baselen);
>  
>  		for (i = 0; i < nparent; ++i) {
>  			/*

Makes sense. And there is a single caller of `path_appendnew()`, only,
so no further changes should be required.

Patrick