On Mon, Jul 21, 2014 at 6:55 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: >> @@ -848,13 +878,21 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, >> strbuf_addf(&sb_repo, "%d", counter); >> } >> name = strrchr(sb_repo.buf, '/') + 1; >> + >> + junk_pid = getpid(); >> + atexit(remove_junk); >> + sigchain_push_common(remove_junk_on_signal); >> + >> if (mkdir(sb_repo.buf, 0777)) >> die_errno(_("could not create directory of '%s'"), sb_repo.buf); >> + junk_git_dir = sb_repo.buf; > > I've managed to convince myself that, although junk_git_dir becomes a > dangling pointer by the end of prepare_linked_checkout(), it should > never afterward be accessed. Perhaps it would make sense to make this > easier to follow by clearing junk_git_dir when is_junk is cleared? You're right. And the dangling junk_git_dir can be accessed if "ret" below is non-zero. Will strdup() both junk_*_dir and free them in "if (!ret)" block. Thanks for catching. >> @@ -879,7 +917,14 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, >> memset(&cp, 0, sizeof(cp)); >> cp.git_cmd = 1; >> cp.argv = opts->saved_argv; >> - return run_command(&cp); >> + ret = run_command(&cp); >> + if (!ret) >> + is_junk = 0; > > Here: perhaps also set is_junk_dir to NULL since it otherwise is about > to become a dangling pointer. > >> + strbuf_release(&sb); >> + strbuf_release(&sb_repo); >> + strbuf_release(&sb_git); >> + return ret; -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html