Re: [Junio C Hamano] Re: Consolidate SHA1 object file close

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 17, 2008, 05:01 UTC
Message-ID: <7v4p7s8xjc.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LFD.1.10.0806151057100.2949@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

Show 14 quoted lines

> Actually, thinking about it, the +20 was not just fairly arbitrary and 
> would have been better off with a comment, it was also not a very good 
> number to begin with - it just was the old tempfile pattern rounded up to 
> the next "even" number.
>
> The _correct_ number to use is +39, with a better tempfile pattern.
>
> That needs a few comments, though. So I added those too.
>
> This patch is totally unimportant, but it does mean that a really 
> traditional filesystem can now do the final rename in-place, because the 
> temporary file is not just in the same directory as the final one, but ti 
> also has the same length, so old-style filesystems can literally just edit 
> the name in place and mark the buffer dirty.

That's interesting and somewhat amusing ;-)

Show 5 quoted lines

> So take this or leave it as you want - it's really not very important, I 
> just wrote this because I was thinking about what really goes on at a very 
> low level when we do that final atomic rename to create the actual 
> directory entry. This makes it theoretically just a tiny bit more atomic 
> on old-fashioned filesystems.

However, this may be more important fix. We want to make sure that adjust_shared_perm() is called on the success codepath, especially not when mkdir() does _not_ fail.

diff --git a/sha1_file.c b/sha1_file.c
index 500584b..e300562 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2125,7 +2125,7 @@ static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
 	if (fd < 0 && dirlen) {
 		/* Make sure the directory exists */
 		buffer[dirlen-1] = 0;
-		if (mkdir(buffer, 0777) && adjust_shared_perm(buffer))
+		if (mkdir(buffer, 0777) || adjust_shared_perm(buffer))
 			return -1;
 
 		/* Try again */