# [PATCH] refs.c: rework ref_locks by abstracting from underlying struct lock_file

2 messages from 2008-01-16 to 2008-01-16. Participants: Brandon Casey.
Thread: https://gitlist.dev/t/11648

## Brandon Casey, 2008-01-16 19:13

Subject: [PATCH] lockfile.c: modify handling of rename failures and alternate_index_output
Message-ID: <478E5762.8000007@nrlssc.navy.mil>
URL: https://gitlist.dev/e/478E5762.8000007%40nrlssc.navy.mil
In-Reply-To: <1200510486-11438-1-git-send-email-casey@nrlssc.navy.mil>

```
commit_lock_file(): if rename() fails, we should not
  truncate the name of the lockfile. This file should be deleted
  by rollback_lock_file() or remove_lock_file().

commit_locked_index(): call close_lock_file() when alternate_index_output
  is set and handle rename() failures like commit_lock_file().
---
 lockfile.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index bcc4786..e8e5dbb 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -171,15 +171,16 @@ int close_lock_file(struct lock_file *lk)
 int commit_lock_file(struct lock_file *lk)
 {
 	char result_file[PATH_MAX];
-	int i;
+	size_t i;
 	if (lk->fd >= 0 && close_lock_file(lk))
 		return -1;
 	strcpy(result_file, lk->filename);
 	i = strlen(result_file) - 5; /* .lock */
 	result_file[i] = 0;
-	i = rename(lk->filename, result_file);
+	if (rename(lk->filename, result_file))
+		return -1;
 	lk->filename[0] = 0;
-	return i;
+	return 0;
 }
 
 int hold_locked_index(struct lock_file *lk, int die_on_error)
@@ -195,9 +196,12 @@ void set_alternate_index_output(const char *name)
 int commit_locked_index(struct lock_file *lk)
 {
 	if (alternate_index_output) {
-		int result = rename(lk->filename, alternate_index_output);
+		if (lk->fd >= 0 && close_lock_file(lk))
+			return -1;
+		if (rename(lk->filename, alternate_index_output))
+			return -1;
 		lk->filename[0] = 0;
-		return result;
+		return 0;
 	}
 	else
 		return commit_lock_file(lk);
-- 
1.5.4.rc3.14.g44397-dirty

```

## Brandon Casey, 2008-01-16 19:14

Subject: [PATCH] refs.c: rework ref_locks by abstracting from underlying struct lock_file
Message-ID: <478E5796.7040001@nrlssc.navy.mil>
URL: https://gitlist.dev/e/478E5796.7040001%40nrlssc.navy.mil
In-Reply-To: <1200510486-11438-2-git-send-email-casey@nrlssc.navy.mil>

```
Instead of calling close_lock_file() and commit_lock_file() directly,
which take a struct lock_file argument, add two new functions:
close_ref() and commit_ref(), which handle calling the previous
lock_file functions and modifying the ref_lock structure.
---
 refs.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index 2c78956..e705ed3 100644
--- a/refs.c
+++ b/refs.c
@@ -1018,6 +1018,22 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 	return 1;
 }
 
+static int close_ref(struct ref_lock *lock)
+{
+	if (close_lock_file(lock->lk))
+		return -1;
+	lock->lock_fd = -1;
+	return 0;
+}
+
+static int commit_ref(struct ref_lock *lock)
+{
+	if (commit_lock_file(lock->lk))
+		return -1;
+	lock->lock_fd = -1;
+	return 0;
+}
+
 void unlock_ref(struct ref_lock *lock)
 {
         /* Do not free lock->lk -- atexit() still looks at them */
@@ -1128,7 +1144,7 @@ int write_ref_sha1(struct ref_lock *lock,
 	}
 	if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
 	    write_in_full(lock->lock_fd, &term, 1) != 1
-	    	|| close_lock_file(lock->lk) < 0) {
+	    	|| close_ref(lock) < 0) {
 		error("Couldn't write %s", lock->lk->filename);
 		unlock_ref(lock);
 		return -1;
@@ -1161,12 +1177,11 @@ int write_ref_sha1(struct ref_lock *lock,
 		    !strcmp(head_ref, lock->ref_name))
 			log_ref_write("HEAD", lock->old_sha1, sha1, logmsg);
 	}
-	if (commit_lock_file(lock->lk)) {
+	if (commit_ref(lock)) {
 		error("Couldn't set %s", lock->ref_name);
 		unlock_ref(lock);
 		return -1;
 	}
-	lock->lock_fd = -1;
 	unlock_ref(lock);
 	return 0;
 }
-- 
1.5.4.rc3.14.g44397-dirty

```
