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

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

[Johannes Schindelin]

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(-)

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

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

[Junio C Hamano]

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.

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

[Johannes Schindelin]

Hi,

On Wed, 25 Jul 2007, Junio C Hamano wrote:

> 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

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

[Junio C Hamano]

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.  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).

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

[Johannes Schindelin]

Hi,

On Thu, 26 Jul 2007, Junio C Hamano wrote:

> 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.

> 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