Thread (42 messages) 42 messages, 3 authors, 2022-08-03

Re: [PATCH v2 4/6] log: fix common "rev.pending" memory leak in "git show"

From: Jeff King <hidden>
Date: 2022-07-29 19:08:17

On Fri, Jul 29, 2022 at 02:55:35PM -0400, Jeff King wrote:
quoted
@@ -744,7 +745,6 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 			rev.shown_one = 1;
 			break;
 		case OBJ_COMMIT:
-			memcpy(&rev.pending, &blank, sizeof(rev.pending));
 			add_object_array(o, name, &rev.pending);
 			ret = cmd_log_walk_no_free(&rev);
 			break;
We now do not do anything to clean up rev.pending. On the first pass,
we'd see our blank pending array and add to it. But on a subsequent pass
(i.e., because we are showing two or more commits), what will we see?

My initial assumption was we'd have the last pass's commit in "pending"
here, so we'd be leaking it. But I think in practice it's OK. We end up
in prepare_revision_walk(), which blanks the list again, and then
processes each element. Non-commits _do_ end up back in the pending
list, which would be a leak. But we know that this code triggers only
for commits, which are placed only in the "commits" list (and that's
cleaned up as we walk it via get_revision()).
Sorry, just one more clarification here.

The "so we'd be leaking it" in the second paragraph is really: so before
this patch, we'd have been leaking it writing over it with blank. After
this patch, we'd be building up an ever-growing list of pending objects,
and showing a quadratic number of entries. ;)

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help