From: Junio C Hamano Date: Sat, 28 Mar 2009 06:14:39 GMT Subject: Re: [PATCH 1/2] Move chmod(foo, 0444) into move_temp_to_file() Message-ID: <7vskky18ds.fsf@gitster.siamese.dyndns.org> In-Reply-To: <200903261616.47185.johan@herland.net> Johan Herland 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 > --- 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 --- 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;