From: Linus Torvalds Date: Sat, 14 Jun 2008 17:50:12 +0000 (-0700) Subject: Avoid cross-directory renames and linking on object creation X-Git-Tag: v1.5.6-rc3~3 X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=5723fe7e;p=git.git Avoid cross-directory renames and linking on object creation Instead of creating new temporary objects in the top-level git object directory, create them in the same directory they will finally end up in anyway. This avoids making the final atomic "rename to stable name" operation be a cross-directory event, which makes it a lot easier for various filesystems. Several filesystems do things like change the inode number when moving files across directories (or refuse to do it entirely). In particular, it can also cause problems for NFS implementations that change the filehandle of a file when it moves to a different directory, like the old user-space NFS server did, and like the Linux knfsd still does if you don't export your filesystems with 'no_subtree_check' or if you export a filesystem that doesn't have stable inode numbers across renames). This change also obviously implies creating the object fan-out subdirectory at tempfile creation time, rather than at the final move_temp_to_file() time. Which actually accounts for most of the size of the patch. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- diff --git a/sha1_file.c b/sha1_file.c index e27c96ee3..8d48a23c0 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2018,49 +2018,12 @@ static void write_sha1_file_prepare(const void *buf, unsigned long len, SHA1_Final(sha1, &c); } -/* - * Link the tempfile to the final place, possibly creating the - * last directory level as you do so. - * - * Returns the errno on failure, 0 on success. - */ -static int link_temp_to_file(const char *tmpfile, const char *filename) -{ - int ret; - char *dir; - - if (!link(tmpfile, filename)) - return 0; - - /* - * Try to mkdir the last path component if that failed. - * - * Re-try the "link()" regardless of whether the mkdir - * succeeds, since a race might mean that somebody - * else succeeded. - */ - ret = errno; - dir = strrchr(filename, '/'); - if (dir) { - *dir = 0; - if (!mkdir(filename, 0777) && adjust_shared_perm(filename)) { - *dir = '/'; - return -2; - } - *dir = '/'; - if (!link(tmpfile, filename)) - return 0; - ret = errno; - } - return ret; -} - /* * Move the just written object into its final resting place */ int move_temp_to_file(const char *tmpfile, const char *filename) { - int ret = link_temp_to_file(tmpfile, filename); + int ret = link(tmpfile, filename); /* * Coda hack - coda doesn't like cross-directory links, @@ -2114,6 +2077,46 @@ static void close_sha1_file(int fd) die("unable to write sha1 file"); } +/* Size of directory component, including the ending '/' */ +static inline int directory_size(const char *filename) +{ + const char *s = strrchr(filename, '/'); + if (!s) + return 0; + return s - filename + 1; +} + +/* + * This creates a temporary file in the same directory as the final + * 'filename' + * + * We want to avoid cross-directory filename renames, because those + * can have problems on various filesystems (FAT, NFS, Coda). + */ +static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename) +{ + int fd, dirlen = directory_size(filename); + + if (dirlen + 20 > bufsiz) { + errno = ENAMETOOLONG; + return -1; + } + memcpy(buffer, filename, dirlen); + strcpy(buffer + dirlen, "tmp_obj_XXXXXX"); + fd = mkstemp(buffer); + if (fd < 0 && dirlen) { + /* Make sure the directory exists */ + buffer[dirlen-1] = 0; + if (mkdir(buffer, 0777) && adjust_shared_perm(buffer)) + return -1; + + /* Try again */ + strcpy(buffer + dirlen - 1, "/tmp_obj_XXXXXX"); + fd = mkstemp(buffer); + } + return fd; +} + static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen, void *buf, unsigned long len, time_t mtime) { @@ -2138,9 +2141,7 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen, return error("sha1 file %s: %s\n", filename, strerror(errno)); } - snprintf(tmpfile, sizeof(tmpfile), "%s/tmp_obj_XXXXXX", get_object_directory()); - - fd = mkstemp(tmpfile); + fd = create_tmpfile(tmpfile, sizeof(tmpfile), filename); if (fd < 0) { if (errno == EPERM) return error("insufficient permission for adding an object to repository database %s\n", get_object_directory());