Re: [PATCH 2-3/3] Name make_*_path functions more accurately

> Rename the make_*_path functions so it's clearer what they do, in
> particlar make clear what the differnce between make_absolute_path and
> make_nonrelative_path is by renaming them real_path and absolute_path
> respectively. make_relative_path has an understandable name and is
> renamed to relative_path to maintain the name convention.
> 
> The function calls have been replaced 1-to-1 in their usage.
> 
> Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
> ---
> 
> This supercedes the 2/3 and 3/3 patches. 
> 
>  abspath.c              |   22 +++++++++++++++++++---
>  builtin/clone.c        |   12 ++++++------
>  builtin/init-db.c      |    8 ++++----
>  builtin/receive-pack.c |    2 +-
>  cache.h                |    6 +++---
>  dir.c                  |    4 ++--
>  environment.c          |    4 ++--
>  exec_cmd.c             |    2 +-
>  lockfile.c             |    4 ++--
>  path.c                 |    2 +-
>  setup.c                |   14 ++++++--------
>  t/t0000-basic.sh       |   10 +++++-----
>  test-path-utils.c      |    4 ++--
>  13 files changed, 54 insertions(+), 40 deletions(-)
> 
> diff --git a/abspath.c b/abspath.c
> index ff14068..47bc73e 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -14,7 +14,14 @@ int is_directory(const char *path)
>  /* We allow "recursive" symbolic links. Only within reason, though. */
>  #define MAXDEPTH 5
>  
> -const char *make_absolute_path(const char *path)
> +/*
> + * Use this to get the real path, i.e. resolve links. If you want an
> + * absolute path but don't mind links, use absolute_path.
> + *
> + * If path is our buffer, then return path, as it's already what the
> + * user wants.
> + */
> +const char *real_path(const char *path)
>  {
>  	static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
>  	char cwd[1024] = "";
> @@ -104,13 +111,22 @@ static const char *get_pwd_cwd(void)
>  	return cwd;
>  }
>  
> -const char *make_nonrelative_path(const char *path)
> +/*
> + * Use this to get an absolute path from a relative one. If you want
> + * to resolve links, you should use real_path.
> + *
> + * If the path is already absolute, then return path. As the user is
> + * never meant to free the return value, we're safe.
> + */
> +const char *absolute_path(const char *path)
>  {
>  	static char buf[PATH_MAX + 1];
>  
>  	if (is_absolute_path(path)) {
> -		if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
> +		if (strlen(path) >= PATH_MAX)
>  			die("Too long path: %.*s", 60, path);
> +		else
> +			return path;
>  	} else {
>  		size_t len;
>  		const char *fmt;

> diff --git a/builtin/clone.c b/builtin/clone.c
> index 60d9a64..780809d 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -100,7 +100,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
>  		path = mkpath("%s%s", repo, suffix[i]);
>  		if (is_directory(path)) {
>  			*is_bundle = 0;
> -			return xstrdup(make_nonrelative_path(path));
> +			return xstrdup(absolute_path(path));
>  		}
>  	}
>  
> @@ -109,7 +109,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
>  		path = mkpath("%s%s", repo, bundle_suffix[i]);
>  		if (!stat(path, &st) && S_ISREG(st.st_mode)) {
>  			*is_bundle = 1;
> -			return xstrdup(make_nonrelative_path(path));
> +			return xstrdup(absolute_path(path));
>  		}
>  	}
>  
> @@ -203,7 +203,7 @@ static void setup_reference(const char *repo)
>  	struct transport *transport;
>  	const struct ref *extra;
>  
> -	ref_git = make_absolute_path(option_reference);
> +	ref_git = real_path(option_reference);
>  
>  	if (is_directory(mkpath("%s/.git/objects", ref_git)))
>  		ref_git = mkpath("%s/.git", ref_git);
> @@ -411,9 +411,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  
>  	path = get_repo_path(repo_name, &is_bundle);
>  	if (path)
> -		repo = xstrdup(make_nonrelative_path(repo_name));
> +		repo = xstrdup(absolute_path(repo_name));
>  	else if (!strchr(repo_name, ':'))
> -		repo = xstrdup(make_absolute_path(repo_name));
> +		repo = xstrdup(real_path(repo_name));
>  	else
>  		repo = repo_name;
>  	is_local = path && !is_bundle;
> @@ -466,7 +466,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  
>  	if (safe_create_leading_directories_const(git_dir) < 0)
>  		die("could not create leading directories of '%s'", git_dir);
> -	set_git_dir(make_absolute_path(git_dir));
> +	set_git_dir(real_path(git_dir));
>  
>  	if (0 <= option_verbosity)
>  		printf("Cloning into %s%s...\n",
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index fbeb380..63cf259 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -501,7 +501,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
>  		const char *git_dir_parent = strrchr(git_dir, '/');
>  		if (git_dir_parent) {
>  			char *rel = xstrndup(git_dir, git_dir_parent - git_dir);
> -			git_work_tree_cfg = xstrdup(make_absolute_path(rel));
> +			git_work_tree_cfg = xstrdup(real_path(rel));
>  			free(rel);
>  		}
>  		if (!git_work_tree_cfg) {
> @@ -510,7 +510,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
>  				die_errno ("Cannot access current working directory");
>  		}
>  		if (work_tree)
> -			set_git_work_tree(make_absolute_path(work_tree));
> +		  set_git_work_tree(real_path(work_tree));
>  		else
>  			set_git_work_tree(git_work_tree_cfg);
>  		if (access(get_git_work_tree(), X_OK))
> @@ -519,10 +519,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
>  	}
>  	else {
>  		if (work_tree)
> -			set_git_work_tree(make_absolute_path(work_tree));
> +			set_git_work_tree(real_path(work_tree));
>  	}
>  
> -	set_git_dir(make_absolute_path(git_dir));
> +	set_git_dir(real_path(git_dir));
>  
>  	return init_db(template_dir, flags);
>  }
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 760817d..d883585 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -740,7 +740,7 @@ static int add_refs_from_alternate(struct alternate_object_database *e, void *un
>  	const struct ref *extra;
>  
>  	e->name[-1] = '\0';
> -	other = xstrdup(make_absolute_path(e->base));
> +	other = xstrdup(real_path(e->base));
>  	e->name[-1] = '/';
>  	len = strlen(other);
>  
> diff --git a/cache.h b/cache.h
> index c7b0a28..dbc87be 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -716,9 +716,9 @@ static inline int is_absolute_path(const char *path)
>  	return path[0] == '/' || has_dos_drive_prefix(path);
>  }
>  int is_directory(const char *);
> -const char *make_absolute_path(const char *path);
> -const char *make_nonrelative_path(const char *path);
> -const char *make_relative_path(const char *abs, const char *base);
> +const char *real_path(const char *path);
> +const char *absolute_path(const char *path);
> +const char *relative_path(const char *abs, const char *base);
>  int normalize_path_copy(char *dst, const char *src);
>  int longest_ancestor_length(const char *path, const char *prefix_list);
>  char *strip_path_suffix(const char *path, const char *suffix);
> diff --git a/dir.c b/dir.c
> index 570b651..5a9372a 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1023,8 +1023,8 @@ char *get_relative_cwd(char *buffer, int size, const char *dir)
>  	if (!getcwd(buffer, size))
>  		die_errno("can't find the current directory");
>  
> -	if (!is_absolute_path(dir))
> -		dir = make_absolute_path(dir);
> +	/* getcwd resolves links and gives us the real path */
> +	dir = real_path(dir);

>  
>  	while (*dir && *dir == *cwd) {
>  		dir++;
> diff --git a/environment.c b/environment.c
> index c3efbb9..cc670b1 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -139,7 +139,7 @@ static int git_work_tree_initialized;
>  void set_git_work_tree(const char *new_work_tree)
>  {
>  	if (git_work_tree_initialized) {
> -		new_work_tree = make_absolute_path(new_work_tree);
> +		new_work_tree = real_path(new_work_tree);
>  		if (strcmp(new_work_tree, work_tree))
>  			die("internal error: work tree has already been set\n"
>  			    "Current worktree: %s\nNew worktree: %s",
> @@ -147,7 +147,7 @@ void set_git_work_tree(const char *new_work_tree)
>  		return;
>  	}
>  	git_work_tree_initialized = 1;
> -	work_tree = xstrdup(make_absolute_path(new_work_tree));
> +	work_tree = xstrdup(real_path(new_work_tree));
>  }
>  
>  const char *get_git_work_tree(void)
> diff --git a/exec_cmd.c b/exec_cmd.c
> index 38545e8..171e841 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -89,7 +89,7 @@ static void add_path(struct strbuf *out, const char *path)
>  		if (is_absolute_path(path))
>  			strbuf_addstr(out, path);
>  		else
> -			strbuf_addstr(out, make_nonrelative_path(path));
> +			strbuf_addstr(out, absolute_path(path));
>  
>  		strbuf_addch(out, PATH_SEP);
>  	}
> diff --git a/lockfile.c b/lockfile.c
> index b0d74cd..c6fb77b 100644
> --- a/lockfile.c
> +++ b/lockfile.c
> @@ -164,10 +164,10 @@ static char *unable_to_lock_message(const char *path, int err)
>  		    "If no other git process is currently running, this probably means a\n"
>  		    "git process crashed in this repository earlier. Make sure no other git\n"
>  		    "process is running and remove the file manually to continue.",
> -			    make_nonrelative_path(path), strerror(err));
> +			    absolute_path(path), strerror(err));
>  	} else
>  		strbuf_addf(&buf, "Unable to create '%s.lock': %s",
> -			    make_nonrelative_path(path), strerror(err));
> +			    absolute_path(path), strerror(err));
>  	return strbuf_detach(&buf, NULL);
>  }
>  
> diff --git a/path.c b/path.c
> index 8951333..4d73cc9 100644
> --- a/path.c
> +++ b/path.c
> @@ -397,7 +397,7 @@ int set_shared_perm(const char *path, int mode)
>  	return 0;
>  }
>  
> -const char *make_relative_path(const char *abs, const char *base)
> +const char *relative_path(const char *abs, const char *base)
>  {
>  	static char buf[PATH_MAX + 1];
>  	int i = 0, j = 0;
> diff --git a/setup.c b/setup.c
> index dadc666..8cb1ad3 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -216,9 +216,7 @@ void setup_work_tree(void)
>  	if (initialized)
>  		return;
>  	work_tree = get_git_work_tree();
> -	git_dir = get_git_dir();
> -	if (!is_absolute_path(git_dir))
> -		git_dir = make_absolute_path(git_dir);
> +	git_dir = real_path(get_git_dir());

>  	if (!work_tree || chdir(work_tree))
>  		die("This operation must be run in a work tree");
>  
> @@ -229,7 +227,7 @@ void setup_work_tree(void)
>  	if (getenv(GIT_WORK_TREE_ENVIRONMENT))
>  		setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
>  
> -	set_git_dir(make_relative_path(git_dir, work_tree));
> +	set_git_dir(relative_path(git_dir, work_tree));
>  	initialized = 1;
>  }
>  
> @@ -309,7 +307,7 @@ const char *read_gitfile_gently(const char *path)
>  
>  	if (!is_git_directory(dir))
>  		die("Not a git repository: %s", dir);
> -	path = make_absolute_path(dir);
> +	path = real_path(dir);
>  
>  	free(buf);
>  	return path;
> @@ -389,7 +387,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,
>  
>  	if (!prefixcmp(cwd, worktree) &&
>  	    cwd[strlen(worktree)] == '/') { /* cwd inside worktree */
> -		set_git_dir(make_absolute_path(gitdirenv));
> +		set_git_dir(real_path(gitdirenv));
>  		if (chdir(worktree))
>  			die_errno("Could not chdir to '%s'", worktree);
>  		cwd[len++] = '/';
> @@ -414,7 +412,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,
>  	/* --work-tree is set without --git-dir; use discovered one */
>  	if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
>  		if (offset != len && !is_absolute_path(gitdir))
> -			gitdir = xstrdup(make_absolute_path(gitdir));
> +			gitdir = xstrdup(real_path(gitdir));
>  		if (chdir(cwd))
>  			die_errno("Could not come back to cwd");
>  		return setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);
> @@ -422,7 +420,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,
>  
>  	/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */
>  	if (is_bare_repository_cfg > 0) {
> -		set_git_dir(offset == len ? gitdir : make_absolute_path(gitdir));
> +		set_git_dir(offset == len ? gitdir : real_path(gitdir));
>  		if (chdir(cwd))
>  			die_errno("Could not come back to cwd");
>  		return NULL;
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index 8deec75..f4e8f43 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -435,7 +435,7 @@ test_expect_success 'update-index D/F conflict' '
>  	test $numpath0 = 1
>  '
>  
> -test_expect_success SYMLINKS 'absolute path works as expected' '
> +test_expect_success SYMLINKS 'real path works as expected' '
>  	mkdir first &&
>  	ln -s ../.git first/.git &&
>  	mkdir second &&
> @@ -443,14 +443,14 @@ test_expect_success SYMLINKS 'absolute path works as expected' '
>  	mkdir third &&
>  	dir="$(cd .git; pwd -P)" &&
>  	dir2=third/../second/other/.git &&
> -	test "$dir" = "$(test-path-utils make_absolute_path $dir2)" &&
> +	test "$dir" = "$(test-path-utils real_path $dir2)" &&
>  	file="$dir"/index &&
> -	test "$file" = "$(test-path-utils make_absolute_path $dir2/index)" &&
> +	test "$file" = "$(test-path-utils real_path $dir2/index)" &&
>  	basename=blub &&
> -	test "$dir/$basename" = "$(cd .git && test-path-utils make_absolute_path "$basename")" &&
> +	test "$dir/$basename" = "$(cd .git && test-path-utils real_path "$basename")" &&
>  	ln -s ../first/file .git/syml &&
>  	sym="$(cd first; pwd -P)"/file &&
> -	test "$sym" = "$(test-path-utils make_absolute_path "$dir2/syml")"
> +	test "$sym" = "$(test-path-utils real_path "$dir2/syml")"
>  '
>  
>  test_expect_success 'very long name in the index handled sanely' '
> diff --git a/test-path-utils.c b/test-path-utils.c
> index d261398..e767159 100644
> --- a/test-path-utils.c
> +++ b/test-path-utils.c
> @@ -11,9 +11,9 @@ int main(int argc, char **argv)
>  		return 0;
>  	}
>  
> -	if (argc >= 2 && !strcmp(argv[1], "make_absolute_path")) {
> +	if (argc >= 2 && !strcmp(argv[1], "real_path")) {
>  		while (argc > 2) {
> -			puts(make_absolute_path(argv[2]));
> +			puts(real_path(argv[2]));
>  			argc--;
>  			argv++;
>  		}