From: Jeff King <peff@peff•net>
To: Patrick Steinhardt <ps@pks•im>
Cc: Git List <git@vger•kernel.org>,
Junio C Hamano <gitster@pobox•com>,
Wink Saville <wink@saville•com>
Subject: Re: [PATCH 08/14] tree-diff: pass whole path string to path_appendnew()
Date: Tue, 14 Jan 2025 04:26:50 -0500 [thread overview]
Message-ID: <20250114092650.GA882468@coredump.intra.peff.net> (raw)
In-Reply-To: <Z4UzyFqao8Ty_RQb@pks.im>
On Mon, Jan 13, 2025 at 04:40:00PM +0100, Patrick Steinhardt wrote:
> 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
next prev parent reply other threads:[~2025-01-14 9:26 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-03 19:28 [BUGREPORT] git diff-tree --cc SEGFAUTs Wink Saville
2025-01-03 20:46 ` Jeff King
2025-01-03 23:34 ` Wink Saville
2025-01-04 0:31 ` Jeff King
2025-01-04 2:55 ` Junio C Hamano
2025-01-04 3:32 ` Jeff King
2025-01-04 18:09 ` Wink Saville
2025-01-05 22:13 ` Wink Saville
2025-01-09 8:27 ` [PATCH 0/14] combine-diff cleanups Jeff King
2025-01-09 8:28 ` [PATCH 01/14] run_diff_files(): delay allocation of combine_diff_path Jeff King
2025-01-09 17:57 ` Junio C Hamano
2025-01-09 8:32 ` [PATCH 02/14] combine-diff: add combine_diff_path_new() Jeff King
2025-01-09 18:05 ` Junio C Hamano
2025-01-13 15:40 ` Patrick Steinhardt
2025-01-14 9:29 ` Jeff King
2025-01-09 8:33 ` [PATCH 03/14] tree-diff: clear parent array in path_appendnew() Jeff King
2025-01-09 18:28 ` Junio C Hamano
2025-01-10 10:54 ` Jeff King
2025-01-09 8:42 ` [PATCH 04/14] combine-diff: use pointer for parent paths Jeff King
2025-01-09 18:49 ` Junio C Hamano
2025-01-09 8:42 ` [PATCH 05/14] diff: add a comment about combine_diff_path.parent.path Jeff King
2025-01-13 15:40 ` Patrick Steinhardt
2025-01-09 8:44 ` [PATCH 06/14] run_diff_files(): de-mystify the size of combine_diff_path struct Jeff King
2025-01-10 16:40 ` Junio C Hamano
2025-01-09 8:46 ` [PATCH 07/14] tree-diff: drop path_appendnew() alloc optimization Jeff King
2025-01-13 15:40 ` Patrick Steinhardt
2025-01-14 10:30 ` Jeff King
2025-01-09 8:49 ` [PATCH 08/14] tree-diff: pass whole path string to path_appendnew() Jeff King
2025-01-13 15:40 ` Patrick Steinhardt
2025-01-14 9:26 ` Jeff King [this message]
2025-01-09 8:49 ` [PATCH 09/14] tree-diff: inline path_appendnew() Jeff King
2025-01-11 0:41 ` Junio C Hamano
2025-01-09 8:50 ` [PATCH 10/14] combine-diff: drop public declaration of combine_diff_path_size() Jeff King
2025-01-09 8:51 ` [PATCH 11/14] tree-diff: drop list-tail argument to diff_tree_paths() Jeff King
2025-01-18 0:33 ` Junio C Hamano
2025-01-09 8:53 ` [PATCH 12/14] tree-diff: use the name "tail" to refer to list tail Jeff King
2025-01-09 8:54 ` [PATCH 13/14] tree-diff: simplify emit_path() list management Jeff King
2025-01-09 8:57 ` [PATCH 14/14] tree-diff: make list tail-passing more explicit Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250114092650.GA882468@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=ps@pks$(echo .)im \
--cc=wink@saville$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox