Re: [PATCH 1/2] Move chmod(foo, 0444) into move_temp_to_file()

[Junio C Hamano <gitster@pobox.com>]

Johan Herland <johan@herland.net> writes:

> When writing out a loose object or a pack (index), move_temp_to_file() is
> called to finalize the resulting file. These files (loose files and packs)
> should all have permission mode 0444 (modulo adjust_shared_perm()).
> Therefore, instead of doing chmod(foo, 0444) explicitly from each callsite
> (or even forgetting to chmod() at all), do the chmod() call from within
> move_temp_to_file().
>
> Signed-off-by: Johan Herland <johan@herland.net>
> ---

I think you would need this on top.

-- >8 --
move_temp_to_file(): do not forget to chmod() in "Coda hack" codepath

Now move_temp_to_file() is responsible for doing everything that is
necessary to turn a tempfile in $GIT_DIR into its final form, it must make
sure "Coda hack" codepath correctly makes the file read-only.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_file.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 3bd20e7..8869488 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2268,7 +2268,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename)
 	 */
 	if (ret && ret != EEXIST) {
 		if (!rename(tmpfile, filename))
-			return 0;
+			goto out;
 		ret = errno;
 	}
 	unlink(tmpfile);
@@ -2279,6 +2279,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename)
 		/* FIXME!!! Collision check here ? */
 	}
 
+out:
 	if (chmod(filename, 0444) || adjust_shared_perm(filename))
 		return error("unable to set permission to '%s'", filename);
 	return 0;