public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Jeff King <peff@peff•net>
To: Git List <git@vger•kernel.org>
Cc: Junio C Hamano <gitster@pobox•com>, Wink Saville <wink@saville•com>
Subject: [PATCH 11/14] tree-diff: drop list-tail argument to diff_tree_paths()
Date: Thu, 9 Jan 2025 03:51:56 -0500	[thread overview]
Message-ID: <20250109085156.GK2748836@coredump.intra.peff.net> (raw)
In-Reply-To: <20250109082723.GA2748497@coredump.intra.peff.net>

The internals of the path diffing code, including ll_diff_tree_paths(),
all take an extra combine_diff_path parameter which they use as the tail
of a list of results, appending any new entries to it.

The public-facing diff_tree_paths() takes the same argument, but it just
makes the callers more awkward. They always start with a clean list, and
have to set up a fake head struct to pass in.

Let's keep the public API clean by always returning a new list. That
keeps the fake struct as an implementation detail of tree-diff.c.

Signed-off-by: Jeff King <peff@peff•net>
---
 combine-diff.c |  9 +++------
 diff.h         |  2 +-
 tree-diff.c    | 14 ++++++++------
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index f21e1f58ba..9527f3160d 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1428,22 +1428,19 @@ static struct combine_diff_path *find_paths_multitree(
 {
 	int i, nparent = parents->nr;
 	const struct object_id **parents_oid;
-	struct combine_diff_path paths_head;
+	struct combine_diff_path *paths;
 	struct strbuf base;
 
 	ALLOC_ARRAY(parents_oid, nparent);
 	for (i = 0; i < nparent; i++)
 		parents_oid[i] = &parents->oid[i];
 
-	/* fake list head, so worker can assume it is non-NULL */
-	paths_head.next = NULL;
-
 	strbuf_init(&base, PATH_MAX);
-	diff_tree_paths(&paths_head, oid, parents_oid, nparent, &base, opt);
+	paths = diff_tree_paths(oid, parents_oid, nparent, &base, opt);
 
 	strbuf_release(&base);
 	free(parents_oid);
-	return paths_head.next;
+	return paths;
 }
 
 static int match_objfind(struct combine_diff_path *path,
diff --git a/diff.h b/diff.h
index 32ad17fd38..7831ed1a2b 100644
--- a/diff.h
+++ b/diff.h
@@ -462,7 +462,7 @@ const char *diff_line_prefix(struct diff_options *);
 extern const char mime_boundary_leader[];
 
 struct combine_diff_path *diff_tree_paths(
-	struct combine_diff_path *p, const struct object_id *oid,
+	const struct object_id *oid,
 	const struct object_id **parents_oid, int nparent,
 	struct strbuf *base, struct diff_options *opt);
 void diff_tree_oid(const struct object_id *old_oid,
diff --git a/tree-diff.c b/tree-diff.c
index 18e5a16716..e99e40da18 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -510,11 +510,14 @@ static struct combine_diff_path *ll_diff_tree_paths(
 }
 
 struct combine_diff_path *diff_tree_paths(
-	struct combine_diff_path *p, const struct object_id *oid,
+	const struct object_id *oid,
 	const struct object_id **parents_oid, int nparent,
 	struct strbuf *base, struct diff_options *opt)
 {
-	p = ll_diff_tree_paths(p, oid, parents_oid, nparent, base, opt, 0);
+	struct combine_diff_path head, *p;
+	/* fake list head, so worker can assume it is non-NULL */
+	head.next = NULL;
+	p = ll_diff_tree_paths(&head, oid, parents_oid, nparent, base, opt, 0);
 	return p;
 }
 
@@ -631,14 +634,13 @@ static void ll_diff_tree_oid(const struct object_id *old_oid,
 			     const struct object_id *new_oid,
 			     struct strbuf *base, struct diff_options *opt)
 {
-	struct combine_diff_path phead, *p;
+	struct combine_diff_path *paths, *p;
 	pathchange_fn_t pathchange_old = opt->pathchange;
 
-	phead.next = NULL;
 	opt->pathchange = emit_diff_first_parent_only;
-	diff_tree_paths(&phead, new_oid, &old_oid, 1, base, opt);
+	paths = diff_tree_paths(new_oid, &old_oid, 1, base, opt);
 
-	for (p = phead.next; p;) {
+	for (p = paths; p;) {
 		struct combine_diff_path *pprev = p;
 		p = p->next;
 		free(pprev);
-- 
2.48.0.rc2.413.gc1c80375a3


  parent reply	other threads:[~2025-01-09  8:51 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
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             ` Jeff King [this message]
2025-01-18  0:33               ` [PATCH 11/14] tree-diff: drop list-tail argument to diff_tree_paths() 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=20250109085156.GK2748836@coredump.intra.peff.net \
    --to=peff@peff$(echo .)net \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --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