[3/4] real_path_if_valid(): remove unsafe API

Commit Message

Linus Arver via GitGitGadget March 6, 2020, 7:03 p.m. UTC

From: Alexandr Miloslavskiy <[e-mail]>

This commit continues the work started with previous commit.

Signed-off-by: Alexandr Miloslavskiy <[e-mail]>
---
 abspath.c   | 10 ----------
 cache.h     |  1 -
 setup.c     |  2 +-
 sha1-file.c | 13 ++++---------
 submodule.c |  7 ++++---
 worktree.c  |  8 ++++++--
 6 files changed, 15 insertions(+), 26 deletions(-)

Comments

Junio C Hamano March 6, 2020, 10:14 p.m. UTC | #1

"Alexandr Miloslavskiy via GitGitGadget" <[e-mail]>
writes:

> diff --git a/sha1-file.c b/sha1-file.c
> index 616886799e5..f2b24654895 100644
> --- a/sha1-file.c
> +++ b/sha1-file.c
> @@ -676,20 +676,15 @@ void add_to_alternates_memory(const char *reference)
>  char *compute_alternate_path(const char *path, struct strbuf *err)
>  {
>  	char *ref_git = NULL;
> -	const char *repo, *ref_git_s;
> +	const char *repo;
>  	int seen_error = 0;
>  
> -	ref_git_s = real_path_if_valid(path);
> -	if (!ref_git_s) {
> +	ref_git = real_pathdup(path, 0);
> +	if (!ref_git) {
>  		seen_error = 1;
>  		strbuf_addf(err, _("path '%s' does not exist"), path);
>  		goto out;
> -	} else
> -		/*
> -		 * Beware: read_gitfile(), real_path() and mkpath()
> -		 * return static buffer
> -		 */
> -		ref_git = xstrdup(ref_git_s);
> +	}

It is amusing to see that rewriting not to use the unsafe function
makes the code a lot easier to follow ;-)

Patch

diff --git a/abspath.c b/abspath.c
index d34026bfeb8..6f15a418bb6 100644
--- a/abspath.c
+++ b/abspath.c
@@ -202,16 +202,6 @@  char *strbuf_realpath(struct strbuf *resolved, const char *path,
 	return retval;
 }
 
-/*
- * Resolve `path` into an absolute, cleaned-up path. The return value
- * comes from a shared buffer.
- */
-const char *real_path_if_valid(const char *path)
-{
-	static struct strbuf realpath = STRBUF_INIT;
-	return strbuf_realpath(&realpath, path, 0);
-}
-
 char *real_pathdup(const char *path, int die_on_error)
 {
 	struct strbuf realpath = STRBUF_INIT;
diff --git a/cache.h b/cache.h
index f6937793ec2..aa3f5ce718a 100644
--- a/cache.h
+++ b/cache.h
@@ -1314,7 +1314,6 @@  static inline int is_absolute_path(const char *path)
 int is_directory(const char *);
 char *strbuf_realpath(struct strbuf *resolved, const char *path,
 		      int die_on_error);
-const char *real_path_if_valid(const char *path);
 char *real_pathdup(const char *path, int die_on_error);
 const char *absolute_path(const char *path);
 char *absolute_pathdup(const char *path);
diff --git a/setup.c b/setup.c
index 19dded55788..d319f499d6b 100644
--- a/setup.c
+++ b/setup.c
@@ -888,7 +888,7 @@  static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_
 
 /*
  * A "string_list_each_func_t" function that canonicalizes an entry
- * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or
+ * from GIT_CEILING_DIRECTORIES using real_pathdup(), or
  * discards it if unusable.  The presence of an empty entry in
  * GIT_CEILING_DIRECTORIES turns off canonicalization for all
  * subsequent entries.
diff --git a/sha1-file.c b/sha1-file.c
index 616886799e5..f2b24654895 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -676,20 +676,15 @@  void add_to_alternates_memory(const char *reference)
 char *compute_alternate_path(const char *path, struct strbuf *err)
 {
 	char *ref_git = NULL;
-	const char *repo, *ref_git_s;
+	const char *repo;
 	int seen_error = 0;
 
-	ref_git_s = real_path_if_valid(path);
-	if (!ref_git_s) {
+	ref_git = real_pathdup(path, 0);
+	if (!ref_git) {
 		seen_error = 1;
 		strbuf_addf(err, _("path '%s' does not exist"), path);
 		goto out;
-	} else
-		/*
-		 * Beware: read_gitfile(), real_path() and mkpath()
-		 * return static buffer
-		 */
-		ref_git = xstrdup(ref_git_s);
+	}
 
 	repo = read_gitfile(ref_git);
 	if (!repo)
diff --git a/submodule.c b/submodule.c
index bad7a788c06..215c62580fc 100644
--- a/submodule.c
+++ b/submodule.c
@@ -2173,7 +2173,7 @@  const char *get_superproject_working_tree(void)
 	static struct strbuf realpath = STRBUF_INIT;
 	struct child_process cp = CHILD_PROCESS_INIT;
 	struct strbuf sb = STRBUF_INIT;
-	const char *one_up = real_path_if_valid("../");
+	struct strbuf one_up = STRBUF_INIT;
 	const char *cwd = xgetcwd();
 	const char *ret = NULL;
 	const char *subpath;
@@ -2188,10 +2188,11 @@  const char *get_superproject_working_tree(void)
 		 */
 		return NULL;
 
-	if (!one_up)
+	if (!strbuf_realpath(&one_up, "../", 0))
 		return NULL;
 
-	subpath = relative_path(cwd, one_up, &sb);
+	subpath = relative_path(cwd, one_up.buf, &sb);
+	strbuf_release(&one_up);
 
 	prepare_submodule_repo_env(&cp.env_array);
 	argv_array_pop(&cp.env_array);
diff --git a/worktree.c b/worktree.c
index e7bbf716f6b..2a340fa939b 100644
--- a/worktree.c
+++ b/worktree.c
@@ -226,17 +226,21 @@  struct worktree *find_worktree(struct worktree **list,
 
 struct worktree *find_worktree_by_path(struct worktree **list, const char *p)
 {
+	struct strbuf wt_path = STRBUF_INIT;
 	char *path = real_pathdup(p, 0);
 
 	if (!path)
 		return NULL;
 	for (; *list; list++) {
-		const char *wt_path = real_path_if_valid((*list)->path);
+		strbuf_reset(&wt_path);
+		if (!strbuf_realpath(&wt_path, (*list)->path, 0))
+			continue;
 
-		if (wt_path && !fspathcmp(path, wt_path))
+		if (!fspathcmp(path, wt_path.buf))
 			break;
 	}
 	free(path);
+	strbuf_release(&wt_path);
 	return *list;
 }