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

[PATCH] Make locked paths absolute when current directory is changed

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Jul 18, 2014, 13:08 UTC
Message-ID
<1405688937-22925-1-git-send-email-pclouds@gmail.com>

Locked paths are saved in a linked list so that if something wrong happens, *.lock are removed. 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 when chdir() is called (or soon to be called, in setup_git_directory_gently case).

Reported-by: Yue Lin Ho <yuelinho777@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 It occurred to me while writing this commit message that it would be
 better if we can unlink via a file descriptor so we don't have to
 convert paths. But I'm not sure about the availability of unlinkat(),
 especially on Windows.
 abspath.c     |  2 +-
 cache.h       |  6 ++++++
 git.c         |  6 +++---
 lockfile.c    | 16 ++++++++++++++++
 path.c        |  4 ++--
 run-command.c |  2 +-
 setup.c       |  3 ++-
 unix-socket.c |  2 +-
 8 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/abspath.c b/abspath.c
index ca33558..78c963f 100644
--- a/abspath.c
+++ b/abspath.c
@@ -87,7 +87,7 @@ static const char *real_path_internal(const char *path, int die_on_error)
 					goto error_out;
 			}
 
-			if (chdir(buf)) {
+			if (chdir_safe(buf)) {
 				if (die_on_error)
 					die_errno("Could not switch to '%s'", buf);
 				else
diff --git a/cache.h b/cache.h
index 44aa439..d3f2596 100644
--- a/cache.h
+++ b/cache.h
@@ -564,6 +564,12 @@ extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
 extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
 extern int commit_lock_file(struct lock_file *);
 extern void update_index_if_able(struct index_state *, struct lock_file *);
+extern void make_locked_paths_absolute(void);
+static inline int chdir_safe(const char *path)
+{
+	make_locked_paths_absolute();
+	return chdir(path);
+}
 
 extern int hold_locked_index(struct lock_file *, int);
 extern int commit_locked_index(struct lock_file *);
diff --git a/git.c b/git.c
index 5b6c761..27766c3 100644
--- a/git.c
+++ b/git.c
@@ -48,7 +48,7 @@ static void save_env(void)
 static void restore_env(void)
 {
 	int i;
-	if (*orig_cwd && chdir(orig_cwd))
+	if (*orig_cwd && chdir_safe(orig_cwd))
 		die_errno("could not move to %s", orig_cwd);
 	for (i = 0; i < ARRAY_SIZE(env_names); i++) {
 		if (orig_env[i])
@@ -206,7 +206,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 				fprintf(stderr, "No directory given for -C.\n" );
 				usage(git_usage_string);
 			}
-			if (chdir((*argv)[1]))
+			if (chdir_safe((*argv)[1]))
 				die_errno("Cannot change to '%s'", (*argv)[1]);
 			if (envchanged)
 				*envchanged = 1;
@@ -292,7 +292,7 @@ static int handle_alias(int *argcp, const char ***argv)
 		ret = 1;
 	}
 
-	if (subdir && chdir(subdir))
+	if (subdir && chdir_safe(subdir))
 		die_errno("Cannot change to '%s'", subdir);
 
 	errno = saved_errno;
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..a70d107 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -280,3 +280,19 @@ void rollback_lock_file(struct lock_file *lk)
 	}
 	lk->filename[0] = 0;
 }
+
+void make_locked_paths_absolute(void)
+{
+	struct lock_file *lk;
+	const char *abspath;
+	for (lk = lock_file_list; lk != NULL; lk = lk->next) {
+		if (!lk->filename[0] || lk->filename[0] == '/')
+			continue;
+		abspath = absolute_path(lk->filename);
+		if (strlen(abspath) >= sizeof(lk->filename))
+			warning("locked path %s is relative when current directory "
+				"is changed", lk->filename);
+		else
+			strcpy(lk->filename, abspath);
+	}
+}
diff --git a/path.c b/path.c
index bc804a3..9e8e101 100644
--- a/path.c
+++ b/path.c
@@ -372,11 +372,11 @@ const char *enter_repo(const char *path, int strict)
 		gitfile = read_gitfile(used_path) ;
 		if (gitfile)
 			strcpy(used_path, gitfile);
-		if (chdir(used_path))
+		if (chdir_safe(used_path))
 			return NULL;
 		path = validated_path;
 	}
-	else if (chdir(path))
+	else if (chdir_safe(path))
 		return NULL;
 
 	if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
diff --git a/run-command.c b/run-command.c
index be07d4a..55f360e 100644
--- a/run-command.c
+++ b/run-command.c
@@ -399,7 +399,7 @@ fail_pipe:
 			close(cmd->out);
 		}
 
-		if (cmd->dir && chdir(cmd->dir))
+		if (cmd->dir && chdir_safe(cmd->dir))
 			die_errno("exec '%s': cd to '%s' failed", cmd->argv[0],
 			    cmd->dir);
 		if (cmd->env) {
diff --git a/setup.c b/setup.c
index 0a22f8b..921045e 100644
--- a/setup.c
+++ b/setup.c
@@ -290,7 +290,7 @@ void setup_work_tree(void)
 	git_dir = get_git_dir();
 	if (!is_absolute_path(git_dir))
 		git_dir = real_path(get_git_dir());
-	if (!work_tree || chdir(work_tree))
+	if (!work_tree || chdir_safe(work_tree))
 		die("This operation must be run in a work tree");
 
 	/*
@@ -636,6 +636,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 		die_errno("Unable to read current working directory");
 	offset = len = strlen(cwd);
 
+	make_locked_paths_absolute();
 	/*
 	 * If GIT_DIR is set explicitly, we're not going
 	 * to do any discovery, but we still do repository
diff --git a/unix-socket.c b/unix-socket.c
index 01f119f..eeb8007 100644
--- a/unix-socket.c
+++ b/unix-socket.c
@@ -12,7 +12,7 @@ static int unix_stream_socket(void)
 static int chdir_len(const char *orig, int len)
 {
 	char *path = xmemdupz(orig, len);
-	int r = chdir(path);
+	int r = chdir_safe(path);
 	free(path);
 	return r;
 }

-- 
1.9.1.346.ga2b5940
Next: Junio C Hamano
Message 1 of 27 in “Make locked paths absolute when current directory is changed”
  1. Make locked paths absolute when current directory is changedNguyễn Thái Ngọc Duy, Jul 18, 2014
  2. Junio C HamanoJul 18, 2014
  3. Duy NguyenJul 19, 2014
  4. Johannes SixtJul 18, 2014
  5. 1/2 lockfile.c: remove PATH_MAX limitation (except in resolve_symlink)Nguyễn Thái Ngọc Duy, Jul 20, 2014
  6. 2/2 Make locked paths absolute when current directory is changedNguyễn Thái Ngọc Duy, Jul 20, 2014
  7. Ramsay JonesJul 21, 2014
  8. Duy NguyenJul 21, 2014
  9. Ramsay JonesJul 21, 2014
  10. Junio C HamanoJul 21, 2014
  11. Duy NguyenJul 23, 2014
  12. Yue Lin HoJul 31, 2014
  13. Duy NguyenJul 31, 2014
  14. Philip OakleyJul 20, 2014
  15. Duy NguyenJul 20, 2014
  16. 0/3 Keep .lock file paths absoluteNguyễn Thái Ngọc Duy, Jul 31, 2014
  17. 1/3 lockfile.c: remove PATH_MAX limitation (except in resolve_symlink)Nguyễn Thái Ngọc Duy, Jul 31, 2014
  18. Junio C HamanoAug 1, 2014
  19. Junio C HamanoAug 1, 2014
  20. Torsten BögershausenAug 2, 2014
  21. Duy NguyenAug 4, 2014
  22. Junio C HamanoAug 4, 2014
  23. Michael HaggertyAug 5, 2014
  24. Yue Lin HoSep 3, 2014
  25. Junio C HamanoAug 1, 2014
  26. 2/3 lockfile.c: remove PATH_MAX limit in resolve_symlink()Nguyễn Thái Ngọc Duy, Jul 31, 2014
  27. 3/3 lockfile.c: store absolute pathNguyễn Thái Ngọc Duy, Jul 31, 2014

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.