git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Bradford C. Smith" <bradford.carl.smith@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Bradford C. Smith" <bradford.carl.smith@gmail.com>
Subject: [PATCH] use lockfile.c routines in git_commit_set_multivar()
Date: Thu, 26 Jul 2007 12:55:28 -0400	[thread overview]
Message-ID: <11854689283208-git-send-email-bradford.carl.smith@gmail.com> (raw)
In-Reply-To: 7vbqe0cazy.fsf@assigned-by-dhcp.cox.net
In-Reply-To: <7vbqe0cazy.fsf@assigned-by-dhcp.cox.net>

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

  reply	other threads:[~2007-07-26 16:55 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-15 21:27 git-config: replaces ~/.gitconfig symlink with real file Bradford Smith
2007-07-15 23:30 ` Johannes Schindelin
2007-07-16  9:37 ` Nikolai Weibull
2007-07-16 11:33   ` Bradford Smith
2007-07-16 13:26     ` Bradford Smith
2007-07-16 22:46       ` Junio C Hamano
2007-07-25 16:49         ` [PATCH 0/2] git-config should not replace symlink Bradford C. Smith
2007-07-25 16:49           ` [PATCH 1/2] resolve symlinks when creating lockfiles Bradford C. Smith
2007-07-25 16:49             ` [PATCH 2/2] use lockfile.c routines in git_commit_set_multivar() Bradford C. Smith
2007-07-25 23:35             ` [PATCH 1/2] resolve symlinks when creating lockfiles Junio C Hamano
2007-07-26 16:55               ` Bradford C. Smith [this message]
2007-07-26 18:31                 ` [PATCH] use lockfile.c routines in git_commit_set_multivar() Johannes Schindelin
2007-07-26 18:48                   ` Bradford Smith
2007-07-27  4:30                     ` Junio C Hamano
2007-07-27  4:53                       ` Junio C Hamano
2007-07-27  9:05                         ` Johannes Schindelin
2007-07-27 18:24                         ` Bradford Smith
2007-07-26 17:34               ` [PATCH] fully resolve symlinks when creating lockfiles Bradford C. Smith
2007-07-26 18:35                 ` Johannes Schindelin
2007-07-26 19:34                 ` Morten Welinder
2007-07-27 16:50                   ` Bradford Smith
2007-07-27  7:05                 ` Junio C Hamano
2007-07-17 13:56       ` git-config: replaces ~/.gitconfig symlink with real file Johannes Schindelin
2007-07-17 14:27         ` Matthieu Moy
2007-07-17 20:35         ` Fredrik Tolf
2007-07-17 20:48           ` Johannes Schindelin
2007-07-17 13:39 ` Catalin Marinas
2007-07-17 16:09   ` Johannes Schindelin

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=11854689283208-git-send-email-bradford.carl.smith@gmail.com \
    --to=bradford.carl.smith@gmail.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).