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

[PATCH] use lockfile.c routines in git_commit_set_multivar()

From
Bradford C. Smith <bradford.carl.smith@gmail.com>
Date
Jul 26, 2007, 16:55 UTC
Message-ID
<11854689283208-git-send-email-bradford.carl.smith@gmail.com>

Changed git_commit_set_multivar() to use the routines provided by lockfile.c to reduce code duplication and ensure consistent behavior.

Signed-off-by: Bradford C. Smith <bradford.carl.smith@gmail.com>
---

I am resubmitting this patch to be considered separately from the symlink resolution change. It differs from the first time I submitted it only in that I have corrected the multi-line comment formatting I used.

I ran the full test suite (make test) on this patch without failures.
 config.c |   30 ++++++++++++++++++------------
 1 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/config.c b/config.c
index f89a611..dd2de6e 100644
--- a/config.c
+++ b/config.c
@@ -715,7 +715,7 @@ int git_config_set_multivar(const char* key, const char* value,
 	int fd = -1, in_fd;
 	int ret;
 	char* config_filename;
-	char* lock_file;
+	struct lock_file *lock = NULL;
 	const char* last_dot = strrchr(key, '.');
 
 	config_filename = getenv(CONFIG_ENVIRONMENT);
@@ -725,7 +725,6 @@ int git_config_set_multivar(const char* key, const char* value,
 			config_filename  = git_path("config");
 	}
 	config_filename = xstrdup(config_filename);
-	lock_file = xstrdup(mkpath("%s.lock", config_filename));
 
 	/*
 	 * Since "key" actually contains the section name and the real
@@ -770,11 +769,12 @@ int git_config_set_multivar(const char* key, const char* value,
 	store.key[i] = 0;
 
 	/*
-	 * The lock_file serves a purpose in addition to locking: the new
+	 * The lock serves a purpose in addition to locking: the new
 	 * contents of .git/config will be written into it.
 	 */
-	fd = open(lock_file, O_WRONLY | O_CREAT | O_EXCL, 0666);
-	if (fd < 0 || adjust_shared_perm(lock_file)) {
+	lock = xcalloc(sizeof(struct lock_file), 1);
+	fd = hold_lock_file_for_update(lock, config_filename, 0);
+	if (fd < 0) {
 		fprintf(stderr, "could not lock config file\n");
 		free(store.key);
 		ret = -1;
@@ -914,25 +914,31 @@ int git_config_set_multivar(const char* key, const char* value,
 				goto write_err_out;
 
 		munmap(contents, contents_sz);
-		unlink(config_filename);
 	}
 
-	if (rename(lock_file, config_filename) < 0) {
-		fprintf(stderr, "Could not rename the lock file?\n");
+	if (close(fd) || commit_lock_file(lock) < 0) {
+		fprintf(stderr, "Cannot commit config file!\n");
 		ret = 4;
 		goto out_free;
 	}
 
+	/* fd is closed, so don't try to close it below. */
+	fd = -1;
+	/*
+	 * lock is committed, so don't try to roll it back below.
+	 * NOTE: Since lockfile.c keeps a linked list of all created
+	 * lock_file structures, it isn't safe to free(lock).  It's
+	 * better to just leave it hanging around.
+	 */
+	lock = NULL;
 	ret = 0;
 
 out_free:
 	if (0 <= fd)
 		close(fd);
+	if (lock)
+		rollback_lock_file(lock);
 	free(config_filename);
-	if (lock_file) {
-		unlink(lock_file);
-		free(lock_file);
-	}
 	return ret;
 
 write_err_out:
-- 
1.5.3.rc3.9.g1b487
Next: Johannes Schindelin
Message 1 of 7 in “use lockfile.c routines in git_commit_set_multivar()”
  1. use lockfile.c routines in git_commit_set_multivar()Bradford C. Smith, Jul 26, 2007
  2. Johannes SchindelinJul 26, 2007
  3. Bradford SmithJul 26, 2007
  4. Junio C HamanoJul 27, 2007
  5. Junio C HamanoJul 27, 2007
  6. Johannes SchindelinJul 27, 2007
  7. Bradford SmithJul 27, 2007

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.