With this patch, cmd_clone() safely frees its xstrdup()-allocated memory. Also cmd_clone() registers remove_junk() (for getting called in a signal handler or atexit()) at most once.
junk_work_tree and junk_git_dir (used in remove_junk()) were changed to use static arrays rather than sharing the memory allocated in cmd_clone(). The junk_* global variables are overwritten in each cmd_clone() call. But if concurrent cmd_clone() calls are disallowed that won't be an issue since we only care about the currently executing cmd_clone() in remove_junk().
Signed-off-by: Ali Gholami Rudi <ali@rudi.ir> --- builtin-clone.c | 30 +++++++++++++++++++----------- 1 files changed, 19 insertions(+), 11 deletions(-)
Show changes to builtin-clone.c +19 −11
diff --git a/builtin-clone.c b/builtin-clone.c index 880373f..6ef31cd 100644 --- a/builtin-clone.c +++ b/builtin-clone.c @@ -268,8 +268,9 @@ static const struct ref *clone_local(const char *src_repo, return ret; } -static const char *junk_work_tree; -static const char *junk_git_dir; +static int junk_registered; +static char junk_work_tree[PATH_MAX]; +static char junk_git_dir[PATH_MAX]; static pid_t junk_pid; static void remove_junk(void) @@ -277,12 +278,12 @@ static void remove_junk(void) struct strbuf sb = STRBUF_INIT; if (getpid() != junk_pid) return; - if (junk_git_dir) { + if (*junk_git_dir) { strbuf_addstr(&sb, junk_git_dir); remove_dir_recursively(&sb, 0); strbuf_reset(&sb); } - if (junk_work_tree) { + if (*junk_work_tree) { strbuf_addstr(&sb, junk_work_tree); remove_dir_recursively(&sb, 0); strbuf_reset(&sb); @@ -320,8 +321,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix) { int is_bundle = 0; struct stat buf; - const char *repo_name, *repo, *work_tree, *git_dir; - char *path, *dir; + const char *repo_name, *work_tree; + char *path, *dir, *repo, *git_dir; int dest_exists; const struct ref *refs, *head_points_at, *remote_head, *mapped_refs; struct strbuf key = STRBUF_INIT, value = STRBUF_INIT; @@ -362,7 +363,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) else if (!strchr(repo_name, ':')) repo = xstrdup(make_absolute_path(repo_name)); else - repo = repo_name; + repo = xstrdup(repo_name); if (argc == 2) dir = xstrdup(argv[1]); @@ -393,7 +394,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) } if (!option_bare) { - junk_work_tree = work_tree; + strcpy(junk_work_tree, work_tree); if (safe_create_leading_directories_const(work_tree) < 0) die("could not create leading directories of '%s': %s", work_tree, strerror(errno)); @@ -402,9 +403,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix) work_tree, strerror(errno)); set_git_work_tree(work_tree); } - junk_git_dir = git_dir; - atexit(remove_junk); - sigchain_push_common(remove_junk_on_signal); + strcpy(junk_git_dir, git_dir); + if (!junk_registered) { + junk_registered = 1; + atexit(remove_junk); + sigchain_push_common(remove_junk_on_signal); + } setenv(CONFIG_ENVIRONMENT, mkpath("%s/config", git_dir), 1); @@ -590,6 +594,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix) strbuf_release(&branch_top); strbuf_release(&key); strbuf_release(&value); + free(path); + free(repo); + free(dir); + free(git_dir); junk_pid = 0; return err; }