All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	ramsay@ramsay1.demon.co.uk, yuelinho777@gmail.com,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH v3 3/3] lockfile.c: store absolute path
Date: Thu, 31 Jul 2014 20:43:34 +0700	[thread overview]
Message-ID: <1406814214-21725-4-git-send-email-pclouds@gmail.com> (raw)
In-Reply-To: <1406814214-21725-1-git-send-email-pclouds@gmail.com>

Locked paths can be saved in a linked list so that if something wrong
happens, *.lock are removed. For relative paths, this works fine if we
keep cwd the same, which is true 99% of time except:

- update-index and read-tree hold the lock on $GIT_DIR/index really
  early, then later on may call setup_work_tree() to move cwd.

- Suppose a lock is being held (e.g. by "git add") then somewhere
  down the line, somebody calls real_path (e.g. "link_alt_odb_entry"),
  which temporarily moves cwd away and back.

During that time when cwd is moved (either permanently or temporarily)
and we decide to die(), attempts to remove relative *.lock will fail,
and the next operation will complain that some files are still locked.

Avoid this case by turning relative paths to absolute before storing
the path in "filename" field.

Reported-by: Yue Lin Ho <yuelinho777@gmail.com>
Helped-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 lockfile.c                    | 10 +++++-----
 t/t2107-update-index-basic.sh | 15 +++++++++++++++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index 154915f..0aa70a5 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -117,13 +117,13 @@ static char *resolve_symlink(const char *in)
 
 static int lock_file(struct lock_file *lk, const char *path, int flags)
 {
-	int len;
+	struct strbuf sb = STRBUF_INIT;
+
 	if (!(flags & LOCK_NODEREF) && !(path = resolve_symlink(path)))
 		return -1;
-	len = strlen(path) + 5; /* .lock */
-	lk->filename = xmallocz(len);
-	strcpy(lk->filename, path);
-	strcat(lk->filename, ".lock");
+	strbuf_add_absolute_path(&sb, path);
+	strbuf_addstr(&sb, ".lock");
+	lk->filename = strbuf_detach(&sb, NULL);
 	lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);
 	if (0 <= lk->fd) {
 		if (!lock_file_list) {
diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
index 1bafb90..dfe02f4 100755
--- a/t/t2107-update-index-basic.sh
+++ b/t/t2107-update-index-basic.sh
@@ -65,4 +65,19 @@ test_expect_success '--cacheinfo mode,sha1,path (new syntax)' '
 	test_cmp expect actual
 '
 
+test_expect_success '.lock files cleaned up' '
+	mkdir cleanup &&
+	(
+	cd cleanup &&
+	mkdir worktree &&
+	git init repo &&
+	cd repo &&
+	git config core.worktree ../../worktree &&
+	# --refresh triggers late setup_work_tree,
+	# active_cache_changed is zero, rollback_lock_file fails
+	git update-index --refresh &&
+	! test -f .git/index.lock
+	)
+'
+
 test_done
-- 
2.1.0.rc0.78.gc0d8480

      parent reply	other threads:[~2014-07-31 13:43 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-18 13:08 [PATCH] Make locked paths absolute when current directory is changed Nguyễn Thái Ngọc Duy
2014-07-18 17:47 ` Junio C Hamano
2014-07-19 12:40   ` Duy Nguyen
2014-07-18 20:44 ` Johannes Sixt
2014-07-20 12:13 ` [PATCH v2 1/2] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink) Nguyễn Thái Ngọc Duy
2014-07-20 12:13   ` [PATCH v2 2/2] Make locked paths absolute when current directory is changed Nguyễn Thái Ngọc Duy
2014-07-21 13:27     ` Ramsay Jones
2014-07-21 13:47       ` Duy Nguyen
2014-07-21 14:23         ` Ramsay Jones
2014-07-21 17:04         ` Junio C Hamano
2014-07-23 11:55           ` Duy Nguyen
2014-07-31  3:01             ` Yue Lin Ho
2014-07-31  9:58               ` Duy Nguyen
2014-07-20 12:47   ` [PATCH v2 1/2] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink) Philip Oakley
2014-07-20 12:50     ` Duy Nguyen
2014-07-31 13:43   ` [PATCH v3 0/3] Keep .lock file paths absolute Nguyễn Thái Ngọc Duy
2014-07-31 13:43     ` [PATCH v3 1/3] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink) Nguyễn Thái Ngọc Duy
2014-08-01 16:53       ` Junio C Hamano
2014-08-01 17:55         ` Junio C Hamano
2014-08-02 18:13           ` Torsten Bögershausen
2014-08-04 10:13             ` Duy Nguyen
2014-08-04 17:42               ` Junio C Hamano
2014-08-05 16:10               ` Michael Haggerty
2014-09-03  8:00                 ` Yue Lin Ho
2014-08-01 17:34       ` Junio C Hamano
2014-07-31 13:43     ` [PATCH v3 2/3] lockfile.c: remove PATH_MAX limit in resolve_symlink() Nguyễn Thái Ngọc Duy
2014-07-31 13:43     ` Nguyễn Thái Ngọc Duy [this message]

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=1406814214-21725-4-git-send-email-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ramsay@ramsay1.demon.co.uk \
    --cc=yuelinho777@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.