Better error message when we are unable to lock the index file

Junio C Hamano · Junio C Hamano · commit 40aaae88adfe · 2006-08-12T17:08:25.000-07:00
Most of the callers except the one in refs.c use the function to
update the index file.  Among the index writers, everybody
except write-tree dies if they cannot open it for writing.

This gives the function an extra argument, to tell it to die
when it cannot create a new file as the lockfile.

The only caller that does not have to die is write-tree, because
updating the index for the cache-tree part is optional and not
being able to do so does not affect the correctness.  I think we
do not have to be so careful and make the failure into die() the
same way as other callers, but that would be a different patch.

Signed-off-by: Junio C Hamano &lt;junkio@cox.net&gt;
diff --git a/builtin-add.c b/builtin-add.c
@@ -93,9 +93,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config);
 
-	newfd = hold_lock_file_for_update(&lock_file, get_index_file());
-	if (newfd < 0)
-		die("unable to create new index file");
+	newfd = hold_lock_file_for_update(&lock_file, get_index_file(), 1);
 
 	if (read_cache() < 0)
 		die("index file corrupt");
diff --git a/builtin-apply.c b/builtin-apply.c
@@ -2234,12 +2234,9 @@ static int apply_patch(int fd, const char *filename,
 		apply = 0;
 
 	write_index = check_index && apply;
-	if (write_index && newfd < 0) {
+	if (write_index && newfd < 0)
 		newfd = hold_lock_file_for_update(&lock_file,
-						  get_index_file());
-		if (newfd < 0)
-			die("unable to create new index file");
-	}
+						  get_index_file(), 1);
 	if (check_index) {
 		if (read_cache() < 0)
 			die("unable to read index file");
diff --git a/builtin-mv.c b/builtin-mv.c
@@ -72,10 +72,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config);
 
-	newfd = hold_lock_file_for_update(&lock_file, get_index_file());
-	if (newfd < 0)
-		die("unable to create new index file");
-
+	newfd = hold_lock_file_for_update(&lock_file, get_index_file(), 1);
 	if (read_cache() < 0)
 		die("index file corrupt");
 
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
@@ -884,9 +884,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 
 	git_config(git_default_config);
 
-	newfd = hold_lock_file_for_update(&lock_file, get_index_file());
-	if (newfd < 0)
-		die("unable to create new index file");
+	newfd = hold_lock_file_for_update(&lock_file, get_index_file(), 1);
 
 	git_config(git_default_config);
 
diff --git a/builtin-rm.c b/builtin-rm.c
@@ -52,9 +52,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config);
 
-	newfd = hold_lock_file_for_update(&lock_file, get_index_file());
-	if (newfd < 0)
-		die("unable to create new index file");
+	newfd = hold_lock_file_for_update(&lock_file, get_index_file(), 1);
 
 	if (read_cache() < 0)
 		die("index file corrupt");
diff --git a/builtin-update-index.c b/builtin-update-index.c
@@ -491,9 +491,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 	/* We can't free this memory, it becomes part of a linked list parsed atexit() */
 	lock_file = xcalloc(1, sizeof(struct lock_file));
 
-	newfd = hold_lock_file_for_update(lock_file, get_index_file());
-	if (newfd < 0)
-		die("unable to create new cachefile");
+	newfd = hold_lock_file_for_update(lock_file, get_index_file(), 1);
 
 	entries = read_cache();
 	if (entries < 0)
diff --git a/builtin-write-tree.c b/builtin-write-tree.c
@@ -18,7 +18,7 @@ int write_tree(unsigned char *sha1, int missing_ok, const char *prefix)
 	/* We can't free this memory, it becomes part of a linked list parsed atexit() */
 	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
 
-	newfd = hold_lock_file_for_update(lock_file, get_index_file());
+	newfd = hold_lock_file_for_update(lock_file, get_index_file(), 0);
 
 	entries = read_cache();
 	if (entries < 0)
diff --git a/cache.h b/cache.h
@@ -175,7 +175,7 @@ struct lock_file {
 	struct lock_file *next;
 	char filename[PATH_MAX];
 };
-extern int hold_lock_file_for_update(struct lock_file *, const char *path);
+extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
 extern int commit_lock_file(struct lock_file *);
 extern void rollback_lock_file(struct lock_file *);
 
diff --git a/checkout-index.c b/checkout-index.c
@@ -206,7 +206,7 @@ int main(int argc, char **argv)
 			state.refresh_cache = 1;
 			if (newfd < 0)
 				newfd = hold_lock_file_for_update
-					(&lock_file, get_index_file());
+					(&lock_file, get_index_file(), 1);
 			if (newfd < 0)
 				die("cannot open index.lock file.");
 			continue;
diff --git a/lockfile.c b/lockfile.c
@@ -22,7 +22,7 @@ static void remove_lock_file_on_signal(int signo)
 	raise(signo);
 }
 
-int hold_lock_file_for_update(struct lock_file *lk, const char *path)
+static int lock_file(struct lock_file *lk, const char *path)
 {
 	int fd;
 	sprintf(lk->filename, "%s.lock", path);
@@ -41,6 +41,14 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path)
 	return fd;
 }
 
+int hold_lock_file_for_update(struct lock_file *lk, const char *path, int die_on_error)
+{
+	int fd = lock_file(lk, path);
+	if (fd < 0 && die_on_error)
+		die("unable to create '%s': %s", path, strerror(errno));
+	return fd;
+}
+
 int commit_lock_file(struct lock_file *lk)
 {
 	char result_file[PATH_MAX];
diff --git a/refs.c b/refs.c
@@ -319,13 +319,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *path,
 
 	if (safe_create_leading_directories(lock->ref_file))
 		die("unable to create directory for %s", lock->ref_file);
-	lock->lock_fd = hold_lock_file_for_update(lock->lk, lock->ref_file);
-	if (lock->lock_fd < 0) {
-		error("Couldn't open lock file %s: %s",
-		      lock->lk->filename, strerror(errno));
-		unlock_ref(lock);
-		return NULL;
-	}
+	lock->lock_fd = hold_lock_file_for_update(lock->lk, lock->ref_file, 1);
 
 	return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
 }

Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ static void remove_lock_file_on_signal(int signo)`
`22`	`22`	`raise(signo);`
`23`	`23`	`}`
`24`	`24`
`25`		`-int hold_lock_file_for_update(struct lock_file lk, const char path)`
	`25`	`+static int lock_file(struct lock_file lk, const char path)`
`26`	`26`	`{`
`27`	`27`	`int fd;`
`28`	`28`	`sprintf(lk->filename, "%s.lock", path);`
`@@ -41,6 +41,14 @@ int hold_lock_file_for_update(struct lock_file lk, const char path)`
`41`	`41`	`return fd;`
`42`	`42`	`}`
`43`	`43`
	`44`	`+int hold_lock_file_for_update(struct lock_file lk, const char path, int die_on_error)`
	`45`	`+{`
	`46`	`+ int fd = lock_file(lk, path);`
	`47`	`+ if (fd < 0 && die_on_error)`
	`48`	`+ die("unable to create '%s': %s", path, strerror(errno));`
	`49`	`+ return fd;`
	`50`	`+}`
	`51`	`+`
`44`	`52`	`int commit_lock_file(struct lock_file *lk)`
`45`	`53`	`{`
`46`	`54`	`char result_file[PATH_MAX];`