Skip to content

Commit dd1055e

Browse files
Martin Ågrengitster
authored andcommitted
builtin/commit: fix memory leak in prepare_index()
Release `pathspec` and the string list `partial`. When we clear the string list, make sure we do not free the `util` pointers. That would result in double-freeing, since we set them up as `item->util = item` in `list_paths()`. Initialize the string list early, so that we can always release it. That introduces some unnecessary overhead in various code paths, but means there is one and only one way out of the function. If we ever accumulate more things we need to free, it should be straightforward to do so. Signed-off-by: Martin Ågren <[email protected]> Reviewed-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 94c9fd2 commit dd1055e

File tree

1 file changed

+10
-5
lines changed

1 file changed

+10
-5
lines changed

builtin/commit.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ static void refresh_cache_or_die(int refresh_flags)
336336
static const char *prepare_index(int argc, const char **argv, const char *prefix,
337337
const struct commit *current_head, int is_status)
338338
{
339-
struct string_list partial;
339+
struct string_list partial = STRING_LIST_INIT_DUP;
340340
struct pathspec pathspec;
341341
int refresh_flags = REFRESH_QUIET;
342342
const char *ret;
@@ -381,7 +381,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
381381
warning(_("Failed to update main cache tree"));
382382

383383
commit_style = COMMIT_NORMAL;
384-
return get_lock_file_path(&index_lock);
384+
ret = get_lock_file_path(&index_lock);
385+
goto out;
385386
}
386387

387388
/*
@@ -404,7 +405,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
404405
if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
405406
die(_("unable to write new_index file"));
406407
commit_style = COMMIT_NORMAL;
407-
return get_lock_file_path(&index_lock);
408+
ret = get_lock_file_path(&index_lock);
409+
goto out;
408410
}
409411

410412
/*
@@ -430,7 +432,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
430432
rollback_lock_file(&index_lock);
431433
}
432434
commit_style = COMMIT_AS_IS;
433-
return get_index_file();
435+
ret = get_index_file();
436+
goto out;
434437
}
435438

436439
/*
@@ -461,7 +464,6 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
461464
die(_("cannot do a partial commit during a cherry-pick."));
462465
}
463466

464-
string_list_init(&partial, 1);
465467
if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix, &pathspec))
466468
exit(1);
467469

@@ -491,6 +493,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
491493
discard_cache();
492494
ret = get_lock_file_path(&false_lock);
493495
read_cache_from(ret);
496+
out:
497+
string_list_clear(&partial, 0);
498+
clear_pathspec(&pathspec);
494499
return ret;
495500
}
496501

0 commit comments

Comments
 (0)