[PATCH v4 12/23] log_ref_setup(): separate code for create vs non-create

From

Michael Haggerty <mhagger@alum.mit.edu>

Date

Jan 6, 2017, 16:22 UTC

Message-ID

<06ef507f76f8f894050d7524892de7cd417d1401.1483719289.git.mhagger@alum.mit.edu>

In-Reply-To

<cover.1483719289.git.mhagger@alum.mit.edu>

This also simplifies the next steps.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs/files-backend.c | 59 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index fd8a751..c8f6d82 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2718,45 +2718,64 @@ static int commit_ref(struct ref_lock *lock)
  */
 static int log_ref_setup(const char *refname, struct strbuf *logfile, struct strbuf *err, int force_create)
 {
-	int logfd, oflags = O_APPEND | O_WRONLY;
+	int logfd;
 
 	strbuf_git_path(logfile, "logs/%s", refname);
+
 	if (force_create || should_autocreate_reflog(refname)) {
 		if (safe_create_leading_directories(logfile->buf) < 0) {
 			strbuf_addf(err, "unable to create directory for '%s': "
 				    "%s", logfile->buf, strerror(errno));
 			return -1;
 		}
-		oflags |= O_CREAT;
-	}
-
-	logfd = open(logfile->buf, oflags, 0666);
-	if (logfd < 0) {
-		if (!(oflags & O_CREAT) && (errno == ENOENT || errno == EISDIR))
-			return 0;
+		logfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);
+		if (logfd < 0) {
+			if (errno == EISDIR) {
+				/*
+				 * The directory that is in the way might be
+				 * empty. Try to remove it.
+				 */
+				if (remove_empty_directories(logfile)) {
+					strbuf_addf(err, "there are still logs under "
+						    "'%s'", logfile->buf);
+					return -1;
+				}
+				logfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);
+			}
 
-		if (errno == EISDIR) {
-			if (remove_empty_directories(logfile)) {
-				strbuf_addf(err, "there are still logs under "
-					    "'%s'", logfile->buf);
+			if (logfd < 0) {
+				strbuf_addf(err, "unable to append to '%s': %s",
+					    logfile->buf, strerror(errno));
 				return -1;
 			}
-			logfd = open(logfile->buf, oflags, 0666);
 		}
-
+	} else {
+		logfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);
 		if (logfd < 0) {
-			strbuf_addf(err, "unable to append to '%s': %s",
-				    logfile->buf, strerror(errno));
-			return -1;
+			if (errno == ENOENT || errno == EISDIR) {
+				/*
+				 * The logfile doesn't already exist,
+				 * but that is not an error; it only
+				 * means that we won't write log
+				 * entries to it.
+				 */
+				;
+			} else {
+				strbuf_addf(err, "unable to append to '%s': %s",
+					    logfile->buf, strerror(errno));
+				return -1;
+			}
 		}
 	}
 
-	adjust_shared_perm(logfile->buf);
-	close(logfd);
+	if (logfd >= 0) {
+		adjust_shared_perm(logfile->buf);
+		close(logfd);
+	}
+
 	return 0;
 }
 
-
 static int files_create_reflog(struct ref_store *ref_store,
 			       const char *refname, int force_create,
 			       struct strbuf *err)

-- 
2.9.3