Commit e30d463d authored by Jeff King's avatar Jeff King Committed by Junio C Hamano

reflog-walk: include all fields when freeing complete_reflogs

When we encounter an error adding reflogs for a walk, we try
to free any logs we have read. But we didn't free all
fields, meaning that we could in theory leak all of the
"items" array (which would consitute the bulk of the
allocated memory).

This patch adds a helper which frees all of the entries and
uses it as appropriate.

As it turns out, the leak seems impossible to trigger with
the current code. Of the three error paths that free the
complete_reflogs struct, two only kick in when the items
array is empty, and the third was removed entirely in the
previous commit.

So this patch should be a noop in terms of behavior, but it
fixes a potential maintenance headache should anybody add a
new error path and copy the partial-free code. Which is
what happened in 5026b471 (add_reflog_for_walk: avoid
memory leak, 2017-05-04), though its leaky call was the
third one that was recently removed.
Signed-off-by: default avatarJeff King <[email protected]>
Signed-off-by: default avatarJunio C Hamano <[email protected]>
parent 8aae3cf7
......@@ -38,6 +38,22 @@ static int read_one_reflog(struct object_id *ooid, struct object_id *noid,
return 0;
}
static void free_complete_reflog(struct complete_reflogs *array)
{
int i;
if (!array)
return;
for (i = 0; i < array->nr; i++) {
free(array->items[i].email);
free(array->items[i].message);
}
free(array->items);
free(array->ref);
free(array);
}
static struct complete_reflogs *read_complete_reflog(const char *ref)
{
struct complete_reflogs *reflogs =
......@@ -189,20 +205,14 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
if (ret > 1)
free(b);
else if (ret == 1) {
if (reflogs) {
free(reflogs->ref);
free(reflogs);
}
free_complete_reflog(reflogs);
free(branch);
branch = b;
reflogs = read_complete_reflog(branch);
}
}
if (!reflogs || reflogs->nr == 0) {
if (reflogs) {
free(reflogs->ref);
free(reflogs);
}
free_complete_reflog(reflogs);
free(branch);
return -1;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment