Thread (26 messages) read the whole thread 26 messages, 4 authors, 2025-01-14
STALE564d

[PATCH 01/14] run_diff_files(): delay allocation of combine_diff_path

From: Jeff King <hidden>
Date: 2025-01-09 08:28:20
Subsystem: the rest · Maintainer: Linus Torvalds

Possibly related (same subject, not in this thread)

While looping over the index entries, when we see a higher level stage
the first thing we do is allocate a combine_diff_path struct for it. But
this can leak; if check_removed() returns an error, we'll continue to
the next iteration of the loop without cleaning up.

We can fix this by just delaying the allocation by a few lines.

I don't think this leak is triggered in the test suite, but it's pretty
easy to see by inspection. My ulterior motive here is that the delayed
allocation means we have all of the data needed to initialize "dpath" at
the time of malloc, making it easier to factor out a constructor
function.

Signed-off-by: Jeff King <redacted>
---
 diff-lib.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index c6d3bc4d37..85b8f1fa59 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -156,18 +156,6 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
 			size_t path_len;
 			struct stat st;
 
-			path_len = ce_namelen(ce);
-
-			dpath = xmalloc(combine_diff_path_size(5, path_len));
-			dpath->path = (char *) &(dpath->parent[5]);
-
-			dpath->next = NULL;
-			memcpy(dpath->path, ce->name, path_len);
-			dpath->path[path_len] = '\0';
-			oidclr(&dpath->oid, the_repository->hash_algo);
-			memset(&(dpath->parent[0]), 0,
-			       sizeof(struct combine_diff_parent)*5);
-
 			changed = check_removed(ce, &st);
 			if (!changed)
 				wt_mode = ce_mode_from_stat(ce, st.st_mode);
@@ -178,7 +166,19 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
 				}
 				wt_mode = 0;
 			}
+
+			path_len = ce_namelen(ce);
+
+			dpath = xmalloc(combine_diff_path_size(5, path_len));
+			dpath->path = (char *) &(dpath->parent[5]);
+
+			dpath->next = NULL;
+			memcpy(dpath->path, ce->name, path_len);
+			dpath->path[path_len] = '\0';
+			oidclr(&dpath->oid, the_repository->hash_algo);
 			dpath->mode = wt_mode;
+			memset(&(dpath->parent[0]), 0,
+			       sizeof(struct combine_diff_parent)*5);
 
 			while (i < entries) {
 				struct cache_entry *nce = istate->cache[i];
-- 
2.48.0.rc2.413.gc1c80375a3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help