threads / patch / 9231

patch, 5 partsAdd is_absolute_path(), make_absolute_path() and normalize_path()

Subject: [PATCH 1/5] Add is_absolute_path(), make_absolute_path() and normalize_path()

## tl;dr

5 messages between Jul 26, 2007 and Jul 26, 2007. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Johannes Schindelin· Jul 26, 2007, 06:24 UTC · lore

This patch adds convenience functions to work with absolute paths. The function is_absolute_path() should help the efforts to integrate the MinGW fork.

Note that make_absolute_path() returns a pointer to a static buffer.

Given a path which possibly contains "/../" and "/./", or which end in "/", normalize_path() returns a normalized path.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 cache.h |    6 ++++++
 path.c  |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 0 deletions(-)
Show changes to 2 files +60 −0

cache.h, path.c

diff --git a/cache.h b/cache.h
index 53801b8..b242147 100644
--- a/cache.h
+++ b/cache.h
@@ -358,6 +358,12 @@ int git_config_perm(const char *var, const char *value);
 int adjust_shared_perm(const char *path);
 int safe_create_leading_directories(char *path);
 char *enter_repo(char *path, int strict);
+static inline int is_absolute_path(const char *path)
+{
+	return path[0] == '/';
+}
+const char *make_absolute_path(const char *path);
+char *normalize_path(char *path);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
 extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/path.c b/path.c
index c4ce962..92ce688 100644
--- a/path.c
+++ b/path.c
@@ -292,3 +292,57 @@ int adjust_shared_perm(const char *path)
 		return -2;
 	return 0;
 }
+
+const char *make_absolute_path(const char *path)
+{
+	static char buf[PATH_MAX];
+	const int size = sizeof(buf);
+	int len;
+
+	if (is_absolute_path(path))
+		return path;
+
+	if (!getcwd(buf, size))
+		die ("Could not get current working directory");
+	if (!strcmp(path, "."))
+		return buf;
+
+	len = strlen(buf);
+	if (snprintf(buf + len, size - len, "/%s", path) > size - 1)
+		die ("Could not make absolute path from '%s'", path);
+	return normalize_path(buf);
+}
+
+/* strip out .. and . */
+char *normalize_path(char *path)
+{
+	int i, j;
+
+	for (i = 0, j = 0; path[i]; i++, j++) {
+		if (path[i] == '.') {
+			if (path[i + 1] == '/') {
+				i++; j--;
+				continue;
+			}
+			if (path[i + 1] == '.' && (path[i + 2] == '/' ||
+						!path[i + 2])) {
+				i += 1 + !!path[i + 2];
+				j--;
+				while (j > 0 && path[--j] != '/')
+					; /* do nothing */
+				continue;
+			}
+		}
+		for (; path[i + 1]; i++, j++) {
+			path[j] = path[i];
+			if (path[i] == '/')
+				break;
+		}
+		path[j] = path[i];
+	}
+	if (j > 0 && path[j - 1] == '/')
+		j--;
+	path[j] = '\0';
+
+	return path;
+}

-- 
1.5.3.rc2.42.gda8d-dirty
Junio C Hamano· Jul 26, 2007, 06:53 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/5] Add is_absolute_path(), make_absolute_path() and normalize_path()

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> +/* strip out .. and . */
> +char *normalize_path(char *path)
> +{

This always makes one worry what should happen when foo/../bar is _not_ bar in reality (i.e. foo is symlink to a directory elsewhere).

It depends on what kind of "path" you feed to the function (and its caller, make_absolute_path()). If you always feed a path from the index (or a path obtained by recursively reading a tree), it is Ok. If it is arbitrary path obtained from the user or the filesystem, it is not.

Johannes Schindelin· Jul 26, 2007, 13:58 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/5] Add is_absolute_path(), make_absolute_path() and normalize_path()

Hi,
On Wed, 25 Jul 2007, Junio C Hamano wrote:
Show 15 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > +/* strip out .. and . */
> > +char *normalize_path(char *path)
> > +{
> 
> This always makes one worry what should happen when foo/../bar
> is _not_ bar in reality (i.e. foo is symlink to a directory
> elsewhere).
> 
> It depends on what kind of "path" you feed to the function (and
> its caller, make_absolute_path()).  If you always feed a path
> >from the index (or a path obtained by recursively reading a
> tree), it is Ok.  If it is arbitrary path obtained from the user
> or the filesystem, it is not.
Agree.  Maybe a comment above the function, like
	/*
	 * The function normalize_path() converts ".." and "." names in 
	 * the given path so that "foo/../bar/./" will come out as "bar".
	 *
	 * Note: normalize_path() does not follow symlinks, so if "foo" is
	 * a symlink in the example above, the result will not work as 
	 * expected.
	 */
Hmm?

Ciao, Dscho

Junio C Hamano· Jul 26, 2007, 18:46 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/5] Add is_absolute_path(), make_absolute_path() and normalize_path()

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 12 quoted lines
> Agree.  Maybe a comment above the function, like
>
> 	/*
> 	 * The function normalize_path() converts ".." and "." names in 
> 	 * the given path so that "foo/../bar/./" will come out as "bar".
> 	 *
> 	 * Note: normalize_path() does not follow symlinks, so if "foo" is
> 	 * a symlink in the example above, the result will not work as 
> 	 * expected.
> 	 */
>
> Hmm?

That comment only states the obvious and does not give a clue to the callers when it should not be used, I am afraid. For example, paths taken out of index or recursively reading trees are Ok because there will not be ".." and "." in them. Making a path given by the user relative to the cwd by prepending what is returned by setup_git_directory() may or may not be safe, depending on how setup_git_directory() does things (I think the original one is safe; I am reasonably sure with the current one when GIT_WORK_TREE is not in use; I do not know when that environment variable is there with the current code with or without your patch series).

Johannes Schindelin· Jul 26, 2007, 19:02 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/5] Add is_absolute_path(), make_absolute_path() and normalize_path()

Hi,
On Thu, 26 Jul 2007, Junio C Hamano wrote:
Show 17 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Agree.  Maybe a comment above the function, like
> >
> > 	/*
> > 	 * The function normalize_path() converts ".." and "." names in 
> > 	 * the given path so that "foo/../bar/./" will come out as "bar".
> > 	 *
> > 	 * Note: normalize_path() does not follow symlinks, so if "foo" is
> > 	 * a symlink in the example above, the result will not work as 
> > 	 * expected.
> > 	 */
> >
> > Hmm?
> 
> That comment only states the obvious and does not give a clue to
> the callers when it should not be used, I am afraid.

I am afraid, that it was unobvious enough to yours truly to forget about that when writing the patch.

Show 8 quoted lines
> For example, paths taken out of index or recursively reading trees are 
> Ok because there will not be ".." and "." in them.  Making a path given 
> by the user relative to the cwd by prepending what is returned by 
> setup_git_directory() may or may not be safe, depending on how 
> setup_git_directory() does things (I think the original one is safe; I 
> am reasonably sure with the current one when GIT_WORK_TREE is not in 
> use; I do not know when that environment variable is there with the 
> current code with or without your patch series).

I am afraid that already GIT_DIR can contain symlinks, and is not checked by setup_git_env().

So I think some concrete comment is needed in _addition_:
	get_git_dir() is not safe, and therefore git_path(), too.
Hmm.

Maybe the easiest way _is_ to getcwd(); chdir() getcwd(); chdir(back); Ugly.

Ciao, Dscho

← back to recent threads