From dd1055ed594f8fef18779cce3cd921c4ac66cf9c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Martin=20=C3=85gren?= Date: Sat, 23 Sep 2017 01:34:49 +0200 Subject: [PATCH] builtin/commit: fix memory leak in `prepare_index()` MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/commit.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 1a0da71a43..ce76b6f960 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -336,7 +336,7 @@ static void refresh_cache_or_die(int refresh_flags) static const char *prepare_index(int argc, const char **argv, const char *prefix, const struct commit *current_head, int is_status) { - struct string_list partial; + struct string_list partial = STRING_LIST_INIT_DUP; struct pathspec pathspec; int refresh_flags = REFRESH_QUIET; const char *ret; @@ -381,7 +381,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix warning(_("Failed to update main cache tree")); commit_style = COMMIT_NORMAL; - return get_lock_file_path(&index_lock); + ret = get_lock_file_path(&index_lock); + goto out; } /* @@ -404,7 +405,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK)) die(_("unable to write new_index file")); commit_style = COMMIT_NORMAL; - return get_lock_file_path(&index_lock); + ret = get_lock_file_path(&index_lock); + goto out; } /* @@ -430,7 +432,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix rollback_lock_file(&index_lock); } commit_style = COMMIT_AS_IS; - return get_index_file(); + ret = get_index_file(); + goto out; } /* @@ -461,7 +464,6 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix die(_("cannot do a partial commit during a cherry-pick.")); } - string_list_init(&partial, 1); if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix, &pathspec)) exit(1); @@ -491,6 +493,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix discard_cache(); ret = get_lock_file_path(&false_lock); read_cache_from(ret); +out: + string_list_clear(&partial, 0); + clear_pathspec(&pathspec); return ret; } -- 2.11.4.GIT