[PATCH v2 1/7] xdiff: clean up read_mmfile() allocations on error
WARM2d
From: Jeff King <hidden>
Date: 2026-09-30 23:44:04
Subsystem:
the rest · Maintainer:
Linus Torvalds
Revision v2 of 2 in this series.
Revisions (2)
- v1 [diff vs current]
- v2 current
When read_mmfile() returns an error, it may or may not have allocated a buffer in the passed-in mmfile_t. So callers must initialize the pointer to NULL and free it even on error. Most callers do this already, but rerere's diff_two() does not, and would leak the buffer after a read error. We could fix it directly, but let's instead try to make the interface less error-prone by freeing the memory when returning failure from read_mmfile(). This fixes (part of) the leak in diff_two(). In theory it also lets us simplify other callers to skip initializing the mmfile. But in practice most still need zero-initialization because they may jump to free() before even calling read_mmfile (e.g., in try_merge()). But we can at least simplify rerere_forget_one_path() a bit. I said "part of" earlier. There's a related leak in diff_two(): if reading the first file succeeds but reading the second fails, we return early and leak the first buffer. We can fix that by checking each individually. Signed-off-by: Jeff King <redacted> --- builtin/rerere.c | 6 +++++- rerere.c | 3 +-- xdiff-interface.c | 1 + 3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/builtin/rerere.c b/builtin/rerere.c
index a056cb791b..d39c6e8445 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c@@ -34,8 +34,12 @@ static int diff_two(const char *file1, const char *label1, mmfile_t minus, plus; int ret; - if (read_mmfile(&minus, file1) || read_mmfile(&plus, file2)) + if (read_mmfile(&minus, file1)) return -1; + if (read_mmfile(&plus, file2)) { + free(minus.ptr); + return -1; + } printf("--- a/%s\n+++ b/%s\n", label1, label2); fflush(stdout);
diff --git a/rerere.c b/rerere.c
index 1c3745d9e3..856347c9ae 100644
--- a/rerere.c
+++ b/rerere.c@@ -1039,7 +1039,7 @@ static int rerere_forget_one_path(struct index_state *istate, for (id->variant = 0; id->variant < id->collection->status_nr; id->variant++) { - mmfile_t cur = { NULL, 0 }; + mmfile_t cur; mmbuffer_t result = {NULL, 0}; int cleanly_resolved;
@@ -1048,7 +1048,6 @@ static int rerere_forget_one_path(struct index_state *istate, handle_cache(istate, path, hash, rerere_path(&buf, id, "thisimage")); if (read_mmfile(&cur, rerere_path(&buf, id, "thisimage"))) { - free(cur.ptr); error(_("failed to update conflicted state in '%s'"), path); goto fail_exit; }
diff --git a/xdiff-interface.c b/xdiff-interface.c
index db6938689f..e3dd2184ae 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c@@ -168,6 +168,7 @@ int read_mmfile(mmfile_t *ptr, const char *filename) sz = xsize_t(st.st_size); ptr->ptr = xmalloc(sz ? sz : 1); if (sz && fread(ptr->ptr, sz, 1, f) != 1) { + FREE_AND_NULL(ptr->ptr); fclose(f); return error("Could not read %s", filename); }
--
2.56.0.354.gb6b32d5be5