git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Johan Herland <johan@herland.net>
Date
Mar 26, 2009, 15:16 UTC
Message-ID
<200903261616.47185.johan@herland.net>
In-Reply-To
<200903261602.37857.johan@herland.net>

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>
---
 fast-import.c |    3 ---
 http-push.c   |    1 -
 http-walker.c |    1 -
 index-pack.c  |    7 +++----
 sha1_file.c   |    3 +--
 5 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index db44da3..23c496d 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -903,9 +903,6 @@ static char *keep_pack(char *curr_index_name)
 	static const char *keep_msg = "fast-import";
 	int keep_fd;
 
-	chmod(pack_data->pack_name, 0444);
-	chmod(curr_index_name, 0444);
-
 	keep_fd = odb_pack_keep(name, sizeof(name), pack_data->sha1);
 	if (keep_fd < 0)
 		die("cannot create keep file");
diff --git a/http-push.c b/http-push.c
index 6ce5a1d..e465b20 100644
--- a/http-push.c
+++ b/http-push.c
@@ -748,7 +748,6 @@ static void finish_request(struct transfer_request *request)
 			aborted = 1;
 		}
 	} else if (request->state == RUN_FETCH_LOOSE) {
-		fchmod(request->local_fileno, 0444);
 		close(request->local_fileno); request->local_fileno = -1;
 
 		if (request->curl_result != CURLE_OK &&
diff --git a/http-walker.c b/http-walker.c
index 0dbad3c..c5a3ea3 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -231,7 +231,6 @@ static void finish_object_request(struct object_request *obj_req)
 {
 	struct stat st;
 
-	fchmod(obj_req->local, 0444);
 	close(obj_req->local); obj_req->local = -1;
 
 	if (obj_req->http_code == 416) {
diff --git a/index-pack.c b/index-pack.c
index 7546822..6e93ee6 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -823,8 +823,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 		}
 		if (move_temp_to_file(curr_pack_name, final_pack_name))
 			die("cannot store pack file");
-	}
-	if (from_stdin)
+	} else if (from_stdin)
 		chmod(final_pack_name, 0444);
 
 	if (final_index_name != curr_index_name) {
@@ -835,8 +834,8 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 		}
 		if (move_temp_to_file(curr_index_name, final_index_name))
 			die("cannot store index file");
-	}
-	chmod(final_index_name, 0444);
+	} else
+		chmod(final_index_name, 0444);
 
 	if (!from_stdin) {
 		printf("%s\n", sha1_to_hex(sha1));
diff --git a/sha1_file.c b/sha1_file.c
index 12e0dfd..87ac53b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2252,7 +2252,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename)
 		/* FIXME!!! Collision check here ? */
 	}
 
-	if (adjust_shared_perm(filename))
+	if (chmod(filename, 0444) || adjust_shared_perm(filename))
 		return error("unable to set permission to '%s'", filename);
 	return 0;
 }
@@ -2278,7 +2278,6 @@ static void close_sha1_file(int fd)
 {
 	if (fsync_object_files)
 		fsync_or_die(fd, "sha1 file");
-	fchmod(fd, 0444);
 	if (close(fd) != 0)
 		die("error when closing sha1 file (%s)", strerror(errno));
 }
-- 
1.6.1.2.461.g5bad6
Previous: Johan HerlandNext: Junio C Hamano
Message 24 of 30 in “[BUG?] How to make a shared/restricted repo?”
  1. Johan HerlandMar 25, 2009
  2. Brandon CaseyMar 25, 2009
  3. Johan HerlandMar 25, 2009
  4. Junio C HamanoMar 25, 2009
  5. Junio C HamanoMar 25, 2009
  6. Johan HerlandMar 25, 2009
  7. Junio C HamanoMar 25, 2009
  8. 0/7 Restricting repository access (Was: [BUG?] How to make a shared/restricted repo?)Johan Herland, Mar 25, 2009
  9. 1/7 Clarify documentation on permissions in shared repositoriesJohan Herland, Mar 25, 2009
  10. 2/7 Cleanup: Remove unnecessary if-else clauseJohan Herland, Mar 25, 2009
  11. 3/7 Introduce core.restrictedRepository for restricting repository permissionsJohan Herland, Mar 25, 2009
  12. 4/7 git-init: Introduce --restricted for restricting repository accessJohan Herland, Mar 25, 2009
  13. 5/7 Add tests for "core.restrictedRepository" and "git init --restricted"Johan Herland, Mar 25, 2009
  14. 6/7 git-init: Apply correct mode bits to template files in shared/restricted repoJohan Herland, Mar 25, 2009
  15. 7/7 Apply restricted permissions to loose objects and pack filesJohan Herland, Mar 25, 2009
  16. Junio C HamanoMar 25, 2009
  17. Johan HerlandMar 26, 2009
  18. Junio C HamanoMar 26, 2009
  19. Johan HerlandMar 26, 2009
  20. Johannes SixtMar 26, 2009
  21. Johan HerlandMar 26, 2009
  22. Johannes SixtMar 26, 2009
  23. 0/2 chmod cleanup (Was: [BUG?] How to make a shared/restricted repo?)Johan Herland, Mar 26, 2009
  24. 1/2 Move chmod(foo, 0444) into move_temp_to_file()Johan Herland, Mar 26, 2009
  25. Junio C HamanoMar 28, 2009
  26. Johan HerlandMar 28, 2009
  27. 2/2 Resolve double chmod() in move_temp_to_file()Johan Herland, Mar 26, 2009
  28. Junio C HamanoMar 28, 2009
  29. Johan HerlandMar 28, 2009
  30. Junio C HamanoMar 29, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.