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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 20, 2014, 23:55 UTC
Message-ID: <CAPig+cRE0xOhQM4mtJQRX_gSQ3_OMr6ws87wU03TZ1eaUXui2g@mail.gmail.com>
In-Reply-To: <1405227068-25506-24-git-send-email-pclouds@gmail.com>

On Sun, Jul 13, 2014 at 12:51 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:

Show 74 quoted lines

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/checkout.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 81adb74..3167fd2 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -20,6 +20,7 @@
>  #include "resolve-undo.h"
>  #include "submodule.h"
>  #include "argv-array.h"
> +#include "sigchain.h"
>
>  static const char * const checkout_usage[] = {
>         N_("git checkout [options] <branch>"),
> @@ -814,6 +815,35 @@ static int switch_branches(const struct checkout_opts *opts,
>         return ret || writeout_error;
>  }
>
> +static const char *junk_work_tree;
> +static const char *junk_git_dir;
> +static int is_junk;
> +static pid_t junk_pid;
> +
> +static void remove_junk(void)
> +{
> +       struct strbuf sb = STRBUF_INIT;
> +       if (!is_junk || getpid() != junk_pid)
> +               return;
> +       if (junk_git_dir) {
> +               strbuf_addstr(&sb, junk_git_dir);
> +               remove_dir_recursively(&sb, 0);
> +               strbuf_reset(&sb);
> +       }
> +       if (junk_work_tree) {
> +               strbuf_addstr(&sb, junk_work_tree);
> +               remove_dir_recursively(&sb, 0);
> +       }
> +       strbuf_release(&sb);
> +}
> +
> +static void remove_junk_on_signal(int signo)
> +{
> +       remove_junk();
> +       sigchain_pop(signo);
> +       raise(signo);
> +}
> +
>  static int prepare_linked_checkout(const struct checkout_opts *opts,
>                                    struct branch_info *new)
>  {
> @@ -822,7 +852,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts,
>         const char *path = opts->new_worktree, *name;
>         struct stat st;
>         struct child_process cp;
> -       int counter = 0, len;
> +       int counter = 0, len, ret;
>
>         if (!new->commit)
>                 die(_("no branch specified"));
> @@ -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?

Show 18 quoted lines

> +       is_junk = 1;
>
>         strbuf_addf(&sb_git, "%s/.git", path);
>         if (safe_create_leading_directories_const(sb_git.buf))
>                 die_errno(_("could not create leading directories of '%s'"),
>                           sb_git.buf);
> +       junk_work_tree = path;
>
>         write_file(sb_git.buf, 1, "gitdir: %s/repos/%s\n",
>                    real_path(get_git_common_dir()), name);
> @@ -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.

Show 10 quoted lines

> +       strbuf_release(&sb);
> +       strbuf_release(&sb_repo);
> +       strbuf_release(&sb_git);
> +       return ret;
> +
>  }
>
>  static int git_checkout_config(const char *var, const char *value, void *cb)
> --
> 1.9.1.346.ga2b5940