git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Kristian Høgsberg" <krh@redhat.com>
To: gitster@pobox.com
Cc: git@vger.kernel.org, "Kristian Høgsberg" <krh@redhat.com>
Subject: [PATCH 1/2] Fix config lockfile handling.
Date: Fri, 14 Dec 2007 14:22:36 -0500	[thread overview]
Message-ID: <1197660157-24109-2-git-send-email-krh@redhat.com> (raw)
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

  reply	other threads:[~2007-12-14 18:42 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-14 19:22 config.c fixes Kristian Høgsberg
2007-12-14 19:22 ` Kristian Høgsberg [this message]
2007-12-14 19:22   ` [PATCH 2/2] Use a strbuf for building up section header and key/value pair strings Kristian Høgsberg
2007-12-14 19:29   ` [PATCH 1/2] Fix config lockfile handling Johannes Schindelin
2007-12-14 20:07     ` Junio C Hamano
2007-12-14 20:15       ` Johannes Schindelin
2007-12-14 21:18         ` Junio C Hamano
2007-12-14 19:32   ` Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1197660157-24109-2-git-send-email-krh@redhat.com \
    --to=krh@redhat.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).