git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/3] use struct sha1_array in diff_tree_combined()

From
René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date
Dec 17, 2011, 10:15 UTC
Message-ID
<4EEC6BD4.4040302@lsrfire.ath.cx>

Maintaining an array of hashes is easier using sha1_array than open-coding it. This patch also fixes a leak of the SHA1 array in diff_tree_combined_merge().

---
 builtin/diff.c |   12 ++++++------
 combine-diff.c |   34 +++++++++++++---------------------
 diff.h         |    3 ++-
 submodule.c    |   14 +++++---------
 4 files changed, 26 insertions(+), 37 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index 0fe638f..387afa7 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -14,6 +14,7 @@
 #include "log-tree.h"
 #include "builtin.h"
 #include "submodule.h"
+#include "sha1-array.h"
 
 struct blobinfo {
 	unsigned char sha1[20];
@@ -169,7 +170,7 @@ static int builtin_diff_combined(struct rev_info *revs,
 				 struct object_array_entry *ent,
 				 int ents)
 {
-	const unsigned char (*parent)[20];
+	struct sha1_array parents = SHA1_ARRAY_INIT;
 	int i;
 
 	if (argc > 1)
@@ -177,12 +178,11 @@ static int builtin_diff_combined(struct rev_info *revs,
 
 	if (!revs->dense_combined_merges && !revs->combine_merges)
 		revs->dense_combined_merges = revs->combine_merges = 1;
-	parent = xmalloc(ents * sizeof(*parent));
-	for (i = 0; i < ents; i++)
-		hashcpy((unsigned char *)(parent + i), ent[i].item->sha1);
-	diff_tree_combined(parent[0], parent + 1, ents - 1,
+	for (i = 1; i < ents; i++)
+		sha1_array_append(&parents, ent[i].item->sha1);
+	diff_tree_combined(ent[0].item->sha1, &parents,
 			   revs->dense_combined_merges, revs);
-	free((void *)parent);
+	sha1_array_clear(&parents);
 	return 0;
 }
 
diff --git a/combine-diff.c b/combine-diff.c
index 214014d..cfe6230 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -8,6 +8,7 @@
 #include "log-tree.h"
 #include "refs.h"
 #include "userdiff.h"
+#include "sha1-array.h"
 
 static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)
 {
@@ -1116,15 +1117,14 @@ static void handle_combined_callback(struct diff_options *opt,
 }
 
 void diff_tree_combined(const unsigned char *sha1,
-			const unsigned char parent[][20],
-			int num_parent,
+			const struct sha1_array *parents,
 			int dense,
 			struct rev_info *rev)
 {
 	struct diff_options *opt = &rev->diffopt;
 	struct diff_options diffopts;
 	struct combine_diff_path *p, *paths = NULL;
-	int i, num_paths, needsep, show_log_first;
+	int i, num_paths, needsep, show_log_first, num_parent = parents->nr;
 
 	diffopts = *opt;
 	diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
@@ -1144,7 +1144,7 @@ void diff_tree_combined(const unsigned char *sha1,
 			diffopts.output_format = stat_opt;
 		else
 			diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
-		diff_tree_sha1(parent[i], sha1, "", &diffopts);
+		diff_tree_sha1(parents->sha1[i], sha1, "", &diffopts);
 		diffcore_std(&diffopts);
 		paths = intersect_paths(paths, i, num_parent);
 
@@ -1199,22 +1199,14 @@ void diff_tree_combined(const unsigned char *sha1,
 void diff_tree_combined_merge(const unsigned char *sha1,
 			     int dense, struct rev_info *rev)
 {
-	int num_parent;
-	const unsigned char (*parent)[20];
 	struct commit *commit = lookup_commit(sha1);
-	struct commit_list *parents;
-
-	/* count parents */
-	for (parents = commit->parents, num_parent = 0;
-	     parents;
-	     parents = parents->next, num_parent++)
-		; /* nothing */
-
-	parent = xmalloc(num_parent * sizeof(*parent));
-	for (parents = commit->parents, num_parent = 0;
-	     parents;
-	     parents = parents->next, num_parent++)
-		hashcpy((unsigned char *)(parent + num_parent),
-			parents->item->object.sha1);
-	diff_tree_combined(sha1, parent, num_parent, dense, rev);
+	struct commit_list *parent = commit->parents;
+	struct sha1_array parents = SHA1_ARRAY_INIT;
+
+	while (parent) {
+		sha1_array_append(&parents, parent->item->object.sha1);
+		parent = parent->next;
+	}
+	diff_tree_combined(sha1, &parents, dense, rev);
+	sha1_array_clear(&parents);
 }
diff --git a/diff.h b/diff.h
index 0c51724..96085cb 100644
--- a/diff.h
+++ b/diff.h
@@ -12,6 +12,7 @@ struct diff_queue_struct;
 struct strbuf;
 struct diff_filespec;
 struct userdiff_driver;
+struct sha1_array;
 
 typedef void (*change_fn_t)(struct diff_options *options,
 		 unsigned old_mode, unsigned new_mode,
@@ -195,7 +196,7 @@ struct combine_diff_path {
 extern void show_combined_diff(struct combine_diff_path *elem, int num_parent,
 			      int dense, struct rev_info *);
 
-extern void diff_tree_combined(const unsigned char *sha1, const unsigned char parent[][20], int num_parent, int dense, struct rev_info *rev);
+extern void diff_tree_combined(const unsigned char *sha1, const struct sha1_array *parents, int dense, struct rev_info *rev);
 
 extern void diff_tree_combined_merge(const unsigned char *sha1, int, struct rev_info *);
 
diff --git a/submodule.c b/submodule.c
index 68c1ba9..788d532 100644
--- a/submodule.c
+++ b/submodule.c
@@ -373,15 +373,11 @@ static void collect_submodules_from_diff(struct diff_queue_struct *q,
 
 static void commit_need_pushing(struct commit *commit, struct commit_list *parent, int *needs_pushing)
 {
-	const unsigned char (*parents)[20];
-	unsigned int i, n;
+	struct sha1_array parents = SHA1_ARRAY_INIT;
 	struct rev_info rev;
 
-	n = commit_list_count(parent);
-	parents = xmalloc(n * sizeof(*parents));
-
-	for (i = 0; i < n; i++) {
-		hashcpy((unsigned char *)(parents + i), parent->item->object.sha1);
+	while (parent) {
+		sha1_array_append(&parents, parent->item->object.sha1);
 		parent = parent->next;
 	}
 
@@ -389,9 +385,9 @@ static void commit_need_pushing(struct commit *commit, struct commit_list *paren
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = collect_submodules_from_diff;
 	rev.diffopt.format_callback_data = needs_pushing;
-	diff_tree_combined(commit->object.sha1, parents, n, 1, &rev);
+	diff_tree_combined(commit->object.sha1, &parents, 1, &rev);
 
-	free((void *)parents);
+	sha1_array_clear(&parents);
 }
 
 int check_submodule_needs_pushing(unsigned char new_sha1[20], const char *remotes_name)
-- 
1.7.8
Next: René Scharfe
Message 1 of 8 in “use struct sha1_array in diff_tree_combined()”
  1. 1/3 use struct sha1_array in diff_tree_combined()René Scharfe, Dec 17, 2011
  2. 2/3 pass struct commit to diff_tree_combined_merge()René Scharfe, Dec 17, 2011
  3. 3/3 submodule: use diff_tree_combined_merge() instead of diff_tree_combined()René Scharfe, Dec 17, 2011
  4. René ScharfeDec 17, 2011
  5. René ScharfeDec 17, 2011
  6. Jeff KingDec 17, 2011
  7. René ScharfeDec 17, 2011
  8. Jeff KingDec 17, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.