clone: initialize atexit cleanup handler earlier
authorJeff King <peff@peff.net>
Wed, 18 Mar 2015 18:55:32 +0000 (14:55 -0400)
committerJunio C Hamano <gitster@pobox.com>
Thu, 19 Mar 2015 20:38:07 +0000 (13:38 -0700)
If clone fails, we generally try to clean up any directories
we've created. We do this by installing an atexit handler,
so that we don't have to manually trigger cleanup. However,
since we install this after touching the filesystem, any
errors between our initial mkdir() and our atexit() call
will result in us leaving a crufty directory around.

We can fix this by moving our atexit() call earlier. It's OK
to do it before the junk_work_tree variable is set, because
remove_junk makes sure the variable is initialized. This
means we "activate" the handler by assigning to the
junk_work_tree variable, which we now bump down to just
after we call mkdir(). We probably do not want to do it
before, because a plausible reason for mkdir() to fail is
EEXIST (i.e., we are racing with another "git init"), and we
would not want to remove their work.

OTOH, this is probably not that big a deal; we will allow
cloning into an empty directory (and skip the mkdir), which
is already racy (i.e., one clone may see the other's empty
dir and start writing into it). Still, it does not hurt to
err on the side of caution here.

Note that writing into junk_work_tree and junk_git_dir after
installing the handler is also technically racy, as we call
our handler on an async signal. Depending on the platform,
we could see a sheared write to the variables. Traditionally
we have not worried about this, and indeed we already do
this later in the function. If we want to address that, it
can come as a separate topic.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/clone.c
index 545105a86fdf07873948f2bbe7d1c4f6e65b1509..ee24162ca47f39d4671734cb5b3be9d5cc5d474e 100644 (file)
@@ -841,20 +841,21 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
                git_dir = mkpathdup("%s/.git", dir);
        }
 
+       atexit(remove_junk);
+       sigchain_push_common(remove_junk_on_signal);
+
        if (!option_bare) {
-               junk_work_tree = work_tree;
                if (safe_create_leading_directories_const(work_tree) < 0)
                        die_errno(_("could not create leading directories of '%s'"),
                                  work_tree);
                if (!dest_exists && mkdir(work_tree, 0777))
                        die_errno(_("could not create work tree dir '%s'."),
                                  work_tree);
+               junk_work_tree = work_tree;
                set_git_work_tree(work_tree);
        }
-       junk_git_dir = git_dir;
-       atexit(remove_junk);
-       sigchain_push_common(remove_junk_on_signal);
 
+       junk_git_dir = git_dir;
        if (safe_create_leading_directories_const(git_dir) < 0)
                die(_("could not create leading directories of '%s'"), git_dir);