threads / discuss / 1546

Improve handling of "." and ".." in git-diff-*

Subject: Improve handling of "." and ".." in git-diff-*

## tl;dr

4 messages between Aug 17, 2005 and Aug 23, 2005.

replies: 3people: 2as markdown or json

Linus Torvalds· Aug 17, 2005, 03:44 UTC · lore

This fixes up usage of ".." (without an ending slash) and "." (with or without the ending slash) in the git diff family.

It also fixes pathspec matching for the case of an empty pathspec, since a "." in the top-level directory (or enough ".." under subdirectories) will result in an empty pathspec. We used to not match it against anything, but it should in fact match everything.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
----

This is in addition to the previous fix for the t0000 testcase and applies independently of that.

NOTE! This does _not_ handle ".." or "." in the _middle_ of a pathspec. If you have people who do

	git diff net/../char/
this will not help them - it will _not_ simplify this to
	git diff char/

and only ".." and "." components at the head of the pathspec will be honoured.

Finally, if you try to give a pathspec that goes "outside" of the .git directory (eg, a ".." pattern at the top of the git archive), the code will politely tell you to f*ck off and die (this is not a new feature, the previous code drop did that too, but because we now handle it even for the top-level case without any extra prefix, it's worth noting).

---
diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
--- a/diffcore-pathspec.c
+++ b/diffcore-pathspec.c
@@ -29,6 +29,8 @@ static int matches_pathspec(const char *
 		    name[len] == 0 ||
 		    name[len] == '/')
 			return 1;
+		if (!len)
+			return 1;
 	}
 	return 0;
 }
diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ -191,6 +191,8 @@ int ce_path_match(const struct cache_ent
 			return 1;
 		if (name[matchlen] == '/' || !name[matchlen])
 			return 1;
+		if (!matchlen)
+			return 1;
 	}
 	return 0;
 }
diff --git a/setup.c b/setup.c
--- a/setup.c
+++ b/setup.c
@@ -1,23 +1,60 @@
 #include "cache.h"
 
+static char *prefix_path(const char *prefix, int len, char *path)
+{
+	char *orig = path;
+	for (;;) {
+		char c;
+		if (*path != '.')
+			break;
+		c = path[1];
+		/* "." */
+		if (!c) {
+			path++;
+			break;
+		}
+		/* "./" */
+		if (c == '/') {
+			path += 2;
+			continue;
+		}
+		if (c != '.')
+			break;
+		c = path[2];
+		if (!c)
+			path += 2;
+		else if (c == '/')
+			path += 3;
+		else
+			break;
+		/* ".." and "../" */
+		/* Remove last component of the prefix */
+		do {
+			if (!len)
+				die("'%s' is outside repository", orig);
+			len--;
+		} while (len && prefix[len-1] != '/');
+		continue;
+	}
+	if (len) {
+		int speclen = strlen(path);
+		char *n = xmalloc(speclen + len + 1);
+	
+		memcpy(n, prefix, len);
+		memcpy(n + len, path, speclen+1);
+		path = n;
+	}
+	return path;
+}
+
 const char **get_pathspec(const char *prefix, char **pathspec)
 {
 	char *entry = *pathspec;
 	char **p;
 	int prefixlen;
 
-	if (!prefix) {
-		char **p;
-		if (!entry)
-			return NULL;
-		p = pathspec;
-		do {
-			if (*entry != '.')
-				continue;
-			/* fixup ? */
-		} while ((entry = *++p) != NULL);
-		return (const char **) pathspec;
-	}
+	if (!prefix && !entry)
+		return NULL;
 
 	if (!entry) {
 		static const char *spec[2];
@@ -27,38 +64,10 @@ const char **get_pathspec(const char *pr
 	}
 
 	/* Otherwise we have to re-write the entries.. */
-	prefixlen = strlen(prefix);
 	p = pathspec;
+	prefixlen = prefix ? strlen(prefix) : 0;
 	do {
-		int speclen, len = prefixlen;
-		char *n;
-
-		for (;;) {
-			if (!strcmp(entry, ".")) {
-				entry++;
-				break;
-			}
-			if (!strncmp(entry, "./", 2)) {
-				entry += 2;
-				continue;
-			}
-			if (!strncmp(entry, "../", 3)) {
-				do {
-					if (!len)
-						die("'%s' is outside repository", *p);
-					len--;
-				} while (len && prefix[len-1] != '/');
-				entry += 3;
-				continue;
-			}
-			break;
-		}
-		speclen = strlen(entry);
-		n = xmalloc(speclen + len + 1);
-		
-		memcpy(n, prefix, len);
-		memcpy(n + len, entry, speclen+1);
-		*p = n;
+		*p = prefix_path(prefix, prefixlen, entry);
 	} while ((entry = *++p) != NULL);
 	return (const char **) pathspec;
 }
Linus Torvalds· Aug 17, 2005, 03:52 UTC · re: Linus Torvalds · lore

Re: Improve handling of "." and ".." in git-diff-*

On Tue, 16 Aug 2005, Linus Torvalds wrote:
> 
> This fixes up usage of ".." (without an ending slash) and "." (with or 
> without the ending slash) in the git diff family.
Btw, if it wasn't clear, with this patch you can now do
	git diff .

and it will show the diffs for everything under the current working directory. Similarly, you can do

	git diff ./drivers
or something like
	cd arch/i386
	git diff . ../x86-64 

and it will do the obvious thing (the "obvious thing" in the latter case is to show the diffs for everything under _both_ arch/i386 and arch/x86-64, but not anything outside of that - the path pattern gets rewritten to "arch/i386" and "arch/x86-64").

		Linus

PS. A number of the other "work tree" ops can be similarly extended to DTRT in git project subdirectories, but I wanted to finish off the "git diff" family first, and get that one all done. I think I'm done now, and I might tackle the other things (git-update-cache, git-checkout-cache, git-commit-script, etc) tomorrow.

Alex Riesen· Aug 23, 2005, 19:49 UTC · re: Linus Torvalds · lore

Re: Improve handling of "." and ".." in git-diff-*

On 8/17/05, Linus Torvalds <torvalds@osdl.org> wrote:
> NOTE! This does _not_ handle ".." or "." in the _middle_ of a pathspec. If
> you have people who do
BTW, could this (below) be useful for something?

#include <string.h> #include <stdio.h> #include <stdlib.h>

// an analog of "cd path" from a directory "cwd".
char* pathexpand(const char* cwd, const char* path)
{
    static const char SEP[] = "/";
    if ( !*path ) // empty path -> "." (don't move)
	path = ".";
    if ( !*cwd || *SEP == *path ) // no cwd, or path begins with "/"
	cwd = SEP;
    int len = strlen(cwd);
    char* out = (char*)malloc(len + 1 + strlen(path) + 1);
    char* p = strcpy(out, cwd) + len;
    if ( *SEP != p[-1] )
	*p++ = *SEP;
    for ( ; *path; ++path )
    {
	char * pl = p;
	while ( *path && *SEP != *path )
	    *p++ = *path++;
	*p = '\0';
	if ( p == pl ) /* ..."//"... */
	    ; // just ignore
	/* ..."/./"...  */
	else if ( p - pl == 1 && '.' == *pl )
	    --p; // just ignore
	/* ..."/../"...  */
	else if ( p - pl == 2 && '.' == pl[0] && '.' == pl[1] )
	{
	    // drop last element of the resulting path
	    if ( --pl > out )
		for ( --pl; pl > out && *SEP != *pl; --pl );
	    p = ++pl;
	}
	/* ..."/path/"...  */
	else if ( *path )
	    *p++ = *path; // just add the separator
	if ( !*path )
	    break;
    }
    if ( p > out+1 && *SEP == p[-1] )
	--p;
    *p = '\0';
    return out;
}
#ifdef CHECK_PATHEXPAND
static void check(const char * cwd, const char * path, const char * good)
{
    static int n = 0;
    printf("%-2d: %s$ cd %s", ++n, cwd, path);
    char* t = pathexpand(cwd, path);
    if ( strcmp(t, good) )
	printf(" failed(%s)\n", t);
    else
	printf(" \t\t%s\n", t);
    free(t);
}
int main(int argc, char** argv)
{
    /* 1 */ check("/onelevel", "aa", "/onelevel/aa");
    /* 2 */ check("/", "..", "/");
    /* 3 */ check("/", "../..", "/");
    /* 4 */ check("/one", "aa/../bb", "/one/bb");
    /* 5 */ check("/one/two", "aa//bb", "/one/two/aa/bb");
    /* 6 */ check("", "/aa//bb", "/aa/bb");
    /* 7 */ check("/one/two", "", "/one/two");
    /* 8 */ check("/one/two", "aa/..bb/x/../cc/", "/one/two/aa/..bb/cc");
    /* 9 */ check("/one/two", "aa/x/././cc////", "/one/two/aa/x/cc");
    /* 10 */ check("/one/two", "../../../../aa", "/aa");
    return 0;
}
#endif
Alex Riesen· Aug 23, 2005, 19:54 UTC · re: Alex Riesen · lore

Re: Improve handling of "." and ".." in git-diff-*

On 8/23/05, Alex Riesen <raa.lkml@gmail.com> wrote:
Show 6 quoted lines
> On 8/17/05, Linus Torvalds <torvalds@osdl.org> wrote:
> > NOTE! This does _not_ handle ".." or "." in the _middle_ of a pathspec. If
> > you have people who do
> 
> BTW, could this (below) be useful for something?
> 

Well, a bit of explanation is certainly missing, sorry. The code tries to emulate (on purely text level) the behaviour of chdir(2), including stopping at root, going back and forth and removing useless (not changing directory) parts like "/./".

The file in the previous message can be tested with :
  gcc -DCHECK_PATHEXPAND patchexpand.c && ./aout

Cheers, Alex Riesen.

PS: Before anyone asked: the code is mine and free for any use.

← back to recent threads