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

[PATCH 1/2] Fix config lockfile handling.

From
Kristian Høgsberg <krh@redhat.com>
Date
Dec 14, 2007, 19:22 UTC
Message-ID
<1197660157-24109-2-git-send-email-krh@redhat.com>
In-Reply-To
<1197660157-24109-1-git-send-email-krh@redhat.com>

When we commit or roll back the lock file the fd is automatically closed, so don't do that again. Also, just keep the lock on the stack.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 config.c |   22 +++++-----------------
 1 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/config.c b/config.c
index 49d2b42..0725563 100644
--- a/config.c
+++ b/config.c
@@ -754,7 +754,7 @@ int git_config_set_multivar(const char* key, const char* value,
 	int fd = -1, in_fd;
 	int ret;
 	char* config_filename;
-	struct lock_file *lock = NULL;
+	struct lock_file lock;
 	const char* last_dot = strrchr(key, '.');
 
 	config_filename = getenv(CONFIG_ENVIRONMENT);
@@ -811,8 +811,7 @@ int git_config_set_multivar(const char* key, const char* value,
 	 * The lock serves a purpose in addition to locking: the new
 	 * contents of .git/config will be written into it.
 	 */
-	lock = xcalloc(sizeof(struct lock_file), 1);
-	fd = hold_lock_file_for_update(lock, config_filename, 0);
+	fd = hold_lock_file_for_update(&lock, config_filename, 0);
 	if (fd < 0) {
 		fprintf(stderr, "could not lock config file\n");
 		free(store.key);
@@ -955,28 +954,17 @@ int git_config_set_multivar(const char* key, const char* value,
 		munmap(contents, contents_sz);
 	}
 
-	if (close(fd) || commit_lock_file(lock) < 0) {
+	if (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);
+	/* If we already committed the lock file, the rollback is a no-op. */
+	rollback_lock_file(&lock);
 	free(config_filename);
 	return ret;
 
-- 
1.5.3.4
Previous: Kristian HøgsbergNext: Kristian Høgsberg
Message 2 of 8 in “config.c fixes”
  1. Kristian HøgsbergDec 14, 2007
  2. 1/2 Fix config lockfile handling.Kristian Høgsberg, Dec 14, 2007
  3. 2/2 Use a strbuf for building up section header and key/value pair strings.Kristian Høgsberg, Dec 14, 2007
  4. Johannes SchindelinDec 14, 2007
  5. Junio C HamanoDec 14, 2007
  6. Johannes SchindelinDec 14, 2007
  7. Junio C HamanoDec 14, 2007
  8. Johannes SixtDec 14, 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.