Re: [PATCH v7 23/31] checkout: clean up half-prepared directories in --to mode

On Mon, Jul 21, 2014 at 6:55 AM, Eric Sunshine [off-list ref] wrote:

quoted

@@ -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.

quoted

@@ -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.

quoted

+       strbuf_release(&sb);
+       strbuf_release(&sb_repo);
+       strbuf_release(&sb_git);
+       return ret;

-- 
Duy