[PATCH v2 12/14] refs.c: rename log_ref_setup to create_reflog

[Ronnie Sahlberg <sahlberg@google.com>]

log_ref_setup is used to do several semi-related things :
* sometimes it will create a new reflog including missing parent directories
  and cleaning up any conflicting stale directories in the path.
* fill in a filename buffer for the full path to the reflog.
* unconditionally re-adjust the permissions for the file.

This function is only called from two places: checkout.c where it is always
used to create a reflog and from refs.c:log_ref_write where it sometimes is
used to create a reflog and sometimes just used to fill in the filename.

Rename log_ref_setup to create_reflog and change it to only take the
refname as argument to make its signature similar to delete_reflog and
reflog_exists. Change create_reflog to ignore log_all_ref_updates and
"unconditionally" create the reflog when called. Since checkout.c always
wants to create a reflog we can call create_reflog directly and avoid the
temp-and-log_all_ref_update dance.

In log_ref_write, only call create_reflog iff we want to create a reflog
and if the reflog does not yet exist. This means that for the common case
where we want to create a reflog which already exists we now only need to
perform a single lstat() to see that the log already exists instead of
performing, for every single update, open(O_CREAT)+lstat()+close().

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
 builtin/checkout.c |  8 +-----
 refs.c             | 78 ++++++++++++++++++++++++++++++------------------------
 refs.h             |  8 +++---
 3 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 9cbe7d1..c37c4dc 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -597,17 +597,11 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
 	if (opts->new_branch) {
 		if (opts->new_orphan_branch) {
 			if (opts->new_branch_log && !log_all_ref_updates) {
-				int temp;
-				struct strbuf log_file = STRBUF_INIT;
 				int ret;
 				const char *ref_name;
 
 				ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
-				temp = log_all_ref_updates;
-				log_all_ref_updates = 1;
-				ret = log_ref_setup(ref_name, &log_file);
-				log_all_ref_updates = temp;
-				strbuf_release(&log_file);
+				ret = create_reflog(ref_name);
 				if (ret) {
 					fprintf(stderr, _("Can not do reflog for '%s'\n"),
 					    opts->new_orphan_branch);
diff --git a/refs.c b/refs.c
index 4069da1..f579d63 100644
--- a/refs.c
+++ b/refs.c
@@ -2839,54 +2839,60 @@ static int copy_msg(char *buf, const char *msg)
 }
 
 /* This function must set a meaningful errno on failure */
-int log_ref_setup(const char *refname, struct strbuf *logfile)
+int create_reflog(const char *refname)
 {
 	int logfd, oflags = O_APPEND | O_WRONLY;
-
-	strbuf_git_path(logfile, "logs/%s", refname);
-	if (log_all_ref_updates &&
-	    (starts_with(refname, "refs/heads/") ||
-	     starts_with(refname, "refs/remotes/") ||
-	     starts_with(refname, "refs/notes/") ||
-	     !strcmp(refname, "HEAD"))) {
-		if (safe_create_leading_directories(logfile->buf) < 0) {
-			int save_errno = errno;
+	struct strbuf logfile = STRBUF_INIT;
+	int save_errno, ret = 0;
+
+	strbuf_git_path(&logfile, "logs/%s", refname);
+	if (starts_with(refname, "refs/heads/") ||
+	    starts_with(refname, "refs/remotes/") ||
+	    starts_with(refname, "refs/notes/") ||
+	    !strcmp(refname, "HEAD")) {
+		if (safe_create_leading_directories(logfile.buf) < 0) {
+			save_errno = errno;
 			error("unable to create directory for %s",
-			      logfile->buf);
-			errno = save_errno;
-			return -1;
+			      logfile.buf);
+			ret = -1;
+			goto cleanup;
 		}
 		oflags |= O_CREAT;
 	}
 
-	logfd = open(logfile->buf, oflags, 0666);
+	logfd = open(logfile.buf, oflags, 0666);
 	if (logfd < 0) {
-		if (!(oflags & O_CREAT) && errno == ENOENT)
-			return 0;
+		if (!(oflags & O_CREAT) && errno == ENOENT) {
+       			goto cleanup;
+		}
 
 		if ((oflags & O_CREAT) && errno == EISDIR) {
-			if (remove_empty_directories(logfile->buf)) {
-				int save_errno = errno;
+			if (remove_empty_directories(logfile.buf)) {
+				save_errno = errno;
 				error("There are still logs under '%s'",
-				      logfile->buf);
-				errno = save_errno;
-				return -1;
+				      logfile.buf);
+				ret = -1;
+				goto cleanup;
  			}
-			logfd = open(logfile->buf, oflags, 0666);
+			logfd = open(logfile.buf, oflags, 0666);
 		}
 
 		if (logfd < 0) {
-			int save_errno = errno;
+			save_errno = errno;
 			error("Unable to append to %s: %s",
-			      logfile->buf, strerror(errno));
-			errno = save_errno;
-			return -1;
+			      logfile.buf, strerror(errno));
+			ret = -1;
+			goto cleanup;
 		}
 	}
 
-	adjust_shared_perm(logfile->buf);
+	adjust_shared_perm(logfile.buf);
 	close(logfd);
-	return 0;
+ cleanup:
+	strbuf_release(&logfile);
+	if (ret)
+		errno = save_errno;
+	return ret;
 }
 
 static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
@@ -2918,19 +2924,21 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
 static int log_ref_write(const char *refname, const unsigned char *old_sha1,
 			 const unsigned char *new_sha1, const char *msg)
 {
-	int logfd, result, oflags = O_APPEND | O_WRONLY;
+	int logfd, result = 0, oflags = O_APPEND | O_WRONLY;
 	struct strbuf sb_log_file = STRBUF_INIT;
-	const char *log_file;
 
 	if (log_all_ref_updates < 0)
 		log_all_ref_updates = !is_bare_repository();
 
-	result = log_ref_setup(refname, &sb_log_file);
+	if (log_all_ref_updates && !reflog_exists(refname))
+		result = create_reflog(refname);
+
 	if (result)
 		goto done;
-	log_file = sb_log_file.buf;
 
-	logfd = open(log_file, oflags);
+	strbuf_git_path(&sb_log_file, "logs/%s", refname);
+
+	logfd = open(sb_log_file.buf, oflags);
 	if (logfd < 0)
 		goto done;
 	result = log_ref_write_fd(logfd, old_sha1, new_sha1,
@@ -2938,12 +2946,12 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
 	if (result) {
 		int save_errno = errno;
 		close(logfd);
-		error("Unable to append to %s", log_file);
+		error("Unable to append to %s", sb_log_file.buf);
 		errno = save_errno;
 		result = -1;
 	} else if (close(logfd)) {
 		int save_errno = errno;
-		error("Unable to append to %s", log_file);
+		error("Unable to append to %s", sb_log_file.buf);
 		errno = save_errno;
 		result = -1;
 	}
diff --git a/refs.h b/refs.h
index e321721..c19ca82 100644
--- a/refs.h
+++ b/refs.h
@@ -202,11 +202,6 @@ extern int commit_ref(struct ref_lock *lock);
 /** Release any lock taken but not written. **/
 extern void unlock_ref(struct ref_lock *lock);
 
-/*
- * Setup reflog before using. Set errno to something meaningful on failure.
- */
-int log_ref_setup(const char *refname, struct strbuf *logfile);
-
 /** Reads log for the value of ref during at_time. **/
 extern int read_ref_at(const char *refname, unsigned long at_time, int cnt,
 		       unsigned char *sha1, char **msg,
@@ -215,6 +210,9 @@ extern int read_ref_at(const char *refname, unsigned long at_time, int cnt,
 /** Check if a particular reflog exists */
 extern int reflog_exists(const char *refname);
 
+/** Create reflog. Set errno to something meaningful on failure. */
+extern int create_reflog(const char *refname);
+
 /** Delete a reflog */
 extern int delete_reflog(const char *refname);
 
-- 
2.0.0.770.gd892650.dirty