Re: [PATCH v3 1/2] worktree: fix worktree add race.

[Duy Nguyen <pclouds@gmail.com>]

Junio, it seems 2/2 is stuck in an endless discussion. But 1/2 is good
regardless, maybe pick it up now and let 2/2 come later whenever it's
ready?

On Wed, Feb 20, 2019 at 11:16 PM Michal Suchanek <msuchanek@suse.de> wrote:
>
> Git runs a stat loop to find a worktree name that's available and then does
> mkdir on the found name. Turn it to mkdir loop to avoid another invocation of
> worktree add finding the same free name and creating the directory first.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v2:
> - simplify loop exit condition
> - exit early if the mkdir fails for reason other than already present
> worktree
> - make counter unsigned
> ---
>  builtin/worktree.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 3f9907fcc994..85a604cfe98c 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -268,10 +268,10 @@ static int add_worktree(const char *path, const char *refname,
>         struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;
>         struct strbuf sb = STRBUF_INIT;
>         const char *name;
> -       struct stat st;
>         struct child_process cp = CHILD_PROCESS_INIT;
>         struct argv_array child_env = ARGV_ARRAY_INIT;
> -       int counter = 0, len, ret;
> +       unsigned int counter = 0;
> +       int len, ret;
>         struct strbuf symref = STRBUF_INIT;
>         struct commit *commit = NULL;
>         int is_branch = 0;
> @@ -295,8 +295,12 @@ static int add_worktree(const char *path, const char *refname,
>         if (safe_create_leading_directories_const(sb_repo.buf))
>                 die_errno(_("could not create leading directories of '%s'"),
>                           sb_repo.buf);
> -       while (!stat(sb_repo.buf, &st)) {
> +
> +       while (mkdir(sb_repo.buf, 0777)) {
>                 counter++;
> +               if ((errno != EEXIST) || !counter /* overflow */)
> +                       die_errno(_("could not create directory of '%s'"),
> +                                 sb_repo.buf);
>                 strbuf_setlen(&sb_repo, len);
>                 strbuf_addf(&sb_repo, "%d", counter);
>         }
> @@ -306,8 +310,6 @@ static int add_worktree(const char *path, const char *refname,
>         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 = xstrdup(sb_repo.buf);
>         is_junk = 1;
>
> --
> 2.20.1
>


-- 
Duy