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

From: Jeff King <peff@peff.net>
Date: Jan 14, 2025, 09:26 UTC
Message-ID: <20250114092650.GA882468@coredump.intra.peff.net>
In-Reply-To: <Z4UzyFqao8Ty_RQb@pks.im>

On Mon, Jan 13, 2025 at 04:40:00PM +0100, Patrick Steinhardt wrote:

Show 14 quoted lines

> 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.

Sort of. The original took a (ptr,size_t) pair in the form of "base", and then also a (ptr,int) path. That matches what the caller has: "pathlen" comes from tree_entry(), which returns an int (it should probably become a size_t in the long run, but it has a lot of ripple effects if you change it).

Now the caller handles path/pathlen itself here:

> > +		strbuf_add(base, path, pathlen);

So there is nothing left to pass in except a (ptr,size_t) pair. We could have continued passing those in as a strbuf, but calling it "base" doesn't make sense any more.

The "int" is still there, but it just stays in the caller. In the original it becomes a size_t via passing to combine_diff_path_size(). In the new code, it happens when we feed it to strbuf_add().

-Peff