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

[PATCH 09/20] log_ref_setup(): pass the open file descriptor back to the caller

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Feb 16, 2016, 13:22 UTC
Message-ID
<10ca076856c8695723f1f5fae1fee2f38fa778fd.1455626201.git.mhagger@alum.mit.edu>
In-Reply-To
<cover.1455626201.git.mhagger@alum.mit.edu>

This function will most often be called by log_ref_write_1(), which wants to append to the reflog file. In that case, it is silly to close the file only for the caller to reopen it immediately. So, in the case that the file was opened, pass the open file descriptor back to the caller.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs/files-backend.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0cfe1ce..78a6213 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2591,19 +2591,23 @@ static int open_or_create_logfile(const char *path, void *cb)
 }
 
 /*
- * Create a reflog for a ref.  If force_create = 0, the reflog will
- * only be created for certain refs (those for which
- * should_autocreate_reflog returns non-zero.  Otherwise, create it
- * regardless of the ref name.  Fill in *err and return -1 on failure.
+ * Create a reflog for a ref. Store its path to *logfile. If
+ * force_create = 0, only create the reflog for certain refs (those
+ * for which should_autocreate_reflog returns non-zero). Otherwise,
+ * create it regardless of the reference name. If the logfile already
+ * existed or was created, return 0 and set *logfd to the file
+ * descriptor opened for appending to the file. If no logfile exists
+ * and we decided not to create one, return 0 and set *logfd to -1. On
+ * failure, fill in *err and return -1.
  */
-static int log_ref_setup(const char *refname, struct strbuf *logfile, struct strbuf *err, int force_create)
+static int log_ref_setup(const char *refname,
+			 struct strbuf *logfile, int *logfd,
+			 struct strbuf *err, int force_create)
 {
-	int logfd;
-
 	strbuf_git_path(logfile, "logs/%s", refname);
 
 	if (force_create || should_autocreate_reflog(refname)) {
-		if (raceproof_create_file(logfile->buf, open_or_create_logfile, &logfd) < 0) {
+		if (raceproof_create_file(logfile->buf, open_or_create_logfile, logfd) < 0) {
 			if (errno == ENOENT) {
 				strbuf_addf(err, "unable to create directory for %s: "
 					    "%s", logfile->buf, strerror(errno));
@@ -2617,11 +2621,10 @@ static int log_ref_setup(const char *refname, struct strbuf *logfile, struct str
 			return -1;
 		} else {
 			adjust_shared_perm(logfile->buf);
-			close(logfd);
 		}
 	} else {
-		logfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);
-		if (logfd < 0) {
+		*logfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);
+		if (*logfd < 0) {
 			if (errno == ENOENT || errno == EISDIR) {
 				/*
 				 * The logfile doesn't already exist,
@@ -2636,7 +2639,6 @@ static int log_ref_setup(const char *refname, struct strbuf *logfile, struct str
 			}
 		} else {
 			adjust_shared_perm(logfile->buf);
-			close(logfd);
 		}
 	}
 
@@ -2647,8 +2649,11 @@ int safe_create_reflog(const char *refname, int force_create, struct strbuf *err
 {
 	int ret;
 	struct strbuf sb = STRBUF_INIT;
+	int fd;
 
-	ret = log_ref_setup(refname, &sb, err, force_create);
+	ret = log_ref_setup(refname, &sb, &fd, err, force_create);
+	if (!ret && fd >= 0)
+		close(fd);
 	strbuf_release(&sb);
 	return ret;
 }
@@ -2684,17 +2689,17 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
 			   struct strbuf *logfile, int flags,
 			   struct strbuf *err)
 {
-	int logfd, result, oflags = O_APPEND | O_WRONLY;
+	int logfd, result;
 
 	if (log_all_ref_updates < 0)
 		log_all_ref_updates = !is_bare_repository();
 
-	result = log_ref_setup(refname, logfile, err, flags & REF_FORCE_CREATE_REFLOG);
+	result = log_ref_setup(refname, logfile, &logfd, err,
+			       flags & REF_FORCE_CREATE_REFLOG);
 
 	if (result)
 		return result;
 
-	logfd = open(logfile->buf, oflags);
 	if (logfd < 0)
 		return 0;
 	result = log_ref_write_fd(logfd, old_sha1, new_sha1,
-- 
2.7.0
Previous: Junio C HamanoNext: Junio C Hamano
Message 20 of 34 in “Delete directories left empty after ref deletion”
  1. 00/20 Delete directories left empty after ref deletionMichael Haggerty, Feb 16, 2016
  2. 01/20 safe_create_leading_directories_const(): preserve errnoMichael Haggerty, Feb 16, 2016
  3. Jeff KingFeb 16, 2016
  4. 02/20 safe_create_leading_directories(): set errno on SCLD_EXISTSMichael Haggerty, Feb 16, 2016
  5. Junio C HamanoFeb 17, 2016
  6. Michael HaggertyFeb 18, 2016
  7. 03/20 raceproof_create_file(): new functionMichael Haggerty, Feb 16, 2016
  8. Junio C HamanoFeb 17, 2016
  9. 04/20 lock_ref_sha1_basic(): use raceproof_create_file()Michael Haggerty, Feb 16, 2016
  10. Junio C HamanoFeb 17, 2016
  11. 05/20 rename_tmp_log(): use raceproof_create_file()Michael Haggerty, Feb 16, 2016
  12. Junio C HamanoFeb 17, 2016
  13. Michael HaggertyFeb 19, 2016
  14. Junio C HamanoFeb 19, 2016
  15. 06/20 rename_tmp_log(): improve error reportingMichael Haggerty, Feb 16, 2016
  16. Junio C HamanoFeb 18, 2016
  17. 07/20 log_ref_setup(): separate code for create vs non-createMichael Haggerty, Feb 16, 2016
  18. 08/20 log_ref_setup(): improve robustness against racesMichael Haggerty, Feb 16, 2016
  19. Junio C HamanoFeb 18, 2016
  20. 09/20 log_ref_setup(): pass the open file descriptor back to the callerMichael Haggerty, Feb 16, 2016
  21. Junio C HamanoFeb 18, 2016
  22. 10/20 log_ref_write_1(): don't depend on logfileMichael Haggerty, Feb 16, 2016
  23. 11/20 log_ref_setup(): manage the name of the reflog file internallyMichael Haggerty, Feb 16, 2016
  24. 12/20 log_ref_write_1(): inline functionMichael Haggerty, Feb 16, 2016
  25. Junio C HamanoFeb 18, 2016
  26. 13/20 try_remove_empty_parents(): rename parameter "name" -> "refname"Michael Haggerty, Feb 16, 2016
  27. 14/20 try_remove_empty_parents(): don't trash argument contentsMichael Haggerty, Feb 16, 2016
  28. 15/20 try_remove_empty_parents(): don't accommodate consecutive slashesMichael Haggerty, Feb 16, 2016
  29. 16/20 t5505: use "for-each-ref" to test for the non-existence of referencesMichael Haggerty, Feb 16, 2016
  30. 17/20 delete_ref_loose(): derive loose reference path from lockMichael Haggerty, Feb 16, 2016
  31. 18/20 delete_ref_loose(): inline functionMichael Haggerty, Feb 16, 2016
  32. 19/20 try_remove_empty_parents(): teach to remove parents of reflogs, tooMichael Haggerty, Feb 16, 2016
  33. 20/20 ref_transaction_commit(): clean up empty directoriesMichael Haggerty, Feb 16, 2016
  34. Junio C HamanoFeb 17, 2016

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.