[PATCH] Provide a dirname() function when NO_LIBGEN_H=YesPlease

When there is no `libgen.h` to our disposal, we miss the `dirname()`
function.

So far, we only had one user of that function: credential-cache--daemon
(which was only compiled when Unix sockets are available, anyway). But
now we also have `builtin/am.c` as user, so we need it.

Since `dirname()` is a sibling of `basename()`, we simply put our very
own `gitdirname()` implementation next to `gitbasename()` and use it
if `NO_LIBGEN_H` has been set.

Signed-off-by: Johannes Schindelin <redacted>
---

	I stumbled over the compile warning when upgrading Git for Windows
	to 2.6.0. There was a left-over NO_LIBGEN_H=YesPlease (which we
	no longer need in Git for Windows 2.x), but it did point to the
	fact that we use `dirname()` in builtin/am.c now, so we better
	have a fall-back implementation for platforms without libgen.h.

	I tested this implementation a bit, but I still would appreciate
	a few eye-balls to go over it.

 compat/basename.c | 26 ++++++++++++++++++++++++++
 git-compat-util.h |  2 ++
 2 files changed, 28 insertions(+)

diff --git a/compat/basename.c b/compat/basename.c
index d8f8a3c..10dba38 100644
--- a/compat/basename.c
+++ b/compat/basename.c

@@ -13,3 +13,29 @@ char *gitbasename (char *path)
 	}
 	return (char *)base;
 }
+
+char *gitdirname(char *path)
+{
+	char *p = path, *slash, c;
+
+	/* Skip over the disk name in MSDOS pathnames. */
+	if (has_dos_drive_prefix(p))
+		p += 2;
+	/* POSIX.1-2001 says dirname("/") should return "/" */
+	slash = is_dir_sep(*p) ? ++p : NULL;
+	while ((c = *(p++)))
+		if (is_dir_sep(c)) {
+			char *tentative = p - 1;
+
+			/* POSIX.1-2001 says to ignore trailing slashes */
+			while (is_dir_sep(*p))
+				p++;
+			if (*p)
+				slash = tentative;
+		}
+
+	if (!slash)
+		return ".";
+	*slash = '\0';
+	return path;
+}

diff --git a/git-compat-util.h b/git-compat-util.h
index f649e81..8b01aa5 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -253,6 +253,8 @@ struct itimerval {
 #else
 #define basename gitbasename
 extern char *gitbasename(char *);
+#define dirname gitdirname
+extern char *gitdirname(char *);
 #endif
 
 #ifndef NO_ICONV

-- 
2.5.3.windows.1.3.gc322723

Hi Johannes,

On 30/09/15 15:50, Johannes Schindelin wrote:

When there is no `libgen.h` to our disposal, we miss the `dirname()`
function.

So far, we only had one user of that function: credential-cache--daemon
(which was only compiled when Unix sockets are available, anyway). But
now we also have `builtin/am.c` as user, so we need it.

Yes, many moons ago (on my old 32-bit laptop) when I was still 'working'
with MinGW I noticed this same thing while looking into providing a win32
emulation of unix sockets. So, I had to look into this at the same time.
Since this didn't progress, I didn't mention the libgen issue.

Anyway, I still have a 'test-libgen.c' file (attached) from back then that
contains some tests. I don't quite recall what the final state of this
code was, but it was intended to test _existing_ libgen implementations
as well as provide a 'git' version which would work on MinGW, cygwin and
linux. Note that some of the existing implementations didn't all agree on
what the tests should report! I don't remember if I looked at the POSIX
spec or not.

So, I don't know how useful it will be - if nothing else, there are some
tests! :-D

HTH

Ramsay Jones

Hi Ramsay,

On Wed, 30 Sep 2015, Ramsay Jones wrote:

On 30/09/15 15:50, Johannes Schindelin wrote:

quoted

When there is no `libgen.h` to our disposal, we miss the `dirname()`
function.

So far, we only had one user of that function:
credential-cache--daemon (which was only compiled when Unix sockets
are available, anyway). But now we also have `builtin/am.c` as user,
so we need it.

Yes, many moons ago (on my old 32-bit laptop) when I was still 'working'
with MinGW I noticed this same thing while looking into providing a win32
emulation of unix sockets. So, I had to look into this at the same time.
Since this didn't progress, I didn't mention the libgen issue.

Anyway, I still have a 'test-libgen.c' file (attached) from back then that
contains some tests.

Awesome. Thank you! I integrated the tests back into test-path-utils.c
(from where the framework clearly came) and made it part of the regression
test suite in the upcoming v2.

Ciao,
Dscho

Junio Hamano pointed out that there is an implicit assumption in pretty
much all the code calling has_dos_drive_prefix(): it assumes that the
DOS drive prefix is always two bytes long.

While this assumption is pretty safe, we can still make the code more
readable and less error-prone by introducing a function that skips the
DOS drive prefix safely.

While at it, we change the has_dos_drive_prefix() return value: it now
returns the number of bytes to be skipped if there is a DOS drive prefix.

Signed-off-by: Johannes Schindelin <redacted>
---
 compat/basename.c |  4 +---
 compat/mingw.c    | 14 +++++---------
 compat/mingw.h    | 10 +++++++++-
 git-compat-util.h |  8 ++++++++
 path.c            | 14 +++++---------
 5 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/compat/basename.c b/compat/basename.c
index d8f8a3c..9f00421 100644
--- a/compat/basename.c
+++ b/compat/basename.c

@@ -4,9 +4,7 @@
 char *gitbasename (char *path)
 {
 	const char *base;
-	/* Skip over the disk name in MSDOS pathnames. */
-	if (has_dos_drive_prefix(path))
-		path += 2;
+	skip_dos_drive_prefix(&path);
 	for (base = path; *path; path++) {
 		if (is_dir_sep(*path))
 			base = path + 1;

diff --git a/compat/mingw.c b/compat/mingw.c
index 5edea29..1b3530a 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c

@@ -1934,26 +1934,22 @@ pid_t waitpid(pid_t pid, int *status, int options)
 
 int mingw_offset_1st_component(const char *path)
 {
-	int offset = 0;
-	if (has_dos_drive_prefix(path))
-		offset = 2;
+	char *pos = (char *)path;
 
 	/* unc paths */
-	else if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {
-
+	if (!skip_dos_drive_prefix(&pos) &&
+			is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
 		/* skip server name */
-		char *pos = strpbrk(path + 2, "\\/");
+		pos = strpbrk(pos + 2, "\\/");
 		if (!pos)
 			return 0; /* Error: malformed unc path */
 
 		do {
 			pos++;
 		} while (*pos && !is_dir_sep(*pos));
-
-		offset = pos - path;
 	}
 
-	return offset + is_dir_sep(path[offset]);
+	return pos + is_dir_sep(*pos) - path;
 }
 
 int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)

diff --git a/compat/mingw.h b/compat/mingw.h
index 57ca477..b3e5044 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h

@@ -361,7 +361,15 @@ HANDLE winansi_get_osfhandle(int fd);
  * git specific compatibility
  */
 
-#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')
+#define has_dos_drive_prefix(path) \
+	(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
+static inline int mingw_skip_dos_drive_prefix(char **path)
+{
+	int ret = has_dos_drive_prefix(*path);
+	*path += ret;
+	return ret;
+}
+#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix
 #define is_dir_sep(c) ((c) == '/' || (c) == '\\')
 static inline char *mingw_find_last_dir_sep(const char *path)
 {

diff --git a/git-compat-util.h b/git-compat-util.h
index 2da0a75..0d66f3a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -335,6 +335,14 @@ static inline int git_has_dos_drive_prefix(const char *path)
 #define has_dos_drive_prefix git_has_dos_drive_prefix
 #endif
 
+#ifndef skip_dos_drive_prefix
+static inline int git_skip_dos_drive_prefix(const char **path)
+{
+	return 0;
+}
+#define skip_dos_drive_prefix git_skip_dos_drive_prefix
+#endif
+
 #ifndef is_dir_sep
 static inline int git_is_dir_sep(int c)
 {

diff --git a/path.c b/path.c
index 3cd155e..8b7e168 100644
--- a/path.c
+++ b/path.c

@@ -782,13 +782,10 @@ const char *relative_path(const char *in, const char *prefix,
 	else if (!prefix_len)
 		return in;
 
-	if (have_same_root(in, prefix)) {
+	if (have_same_root(in, prefix))
 		/* bypass dos_drive, for "c:" is identical to "C:" */
-		if (has_dos_drive_prefix(in)) {
-			i = 2;
-			j = 2;
-		}
-	} else {
+		i = j = has_dos_drive_prefix(in);
+	else {
 		return in;
 	}
 

@@ -943,11 +940,10 @@ const char *remove_leading_path(const char *in, const char *prefix)
 int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
 {
 	char *dst0;
+	int i;
 
-	if (has_dos_drive_prefix(src)) {
+	for (i = has_dos_drive_prefix(src); i > 0; i--)
 		*dst++ = *src++;
-		*dst++ = *src++;
-	}
 	dst0 = dst;
 
 	if (is_dir_sep(*src)) {

-- 
2.6.3.windows.1.300.g1c25e49

When there is no `libgen.h` to our disposal, we miss the `dirname()`
function.

So far, we only had one user of that function: credential-cache--daemon
(which was only compiled when Unix sockets are available, anyway). But
now we also have `builtin/am.c` as user, so we need it.

Since `dirname()` is a sibling of `basename()`, we simply put our very
own `gitdirname()` implementation next to `gitbasename()` and use it
if `NO_LIBGEN_H` has been set.

Signed-off-by: Johannes Schindelin <redacted>
---
 compat/basename.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 git-compat-util.h |  2 ++
 2 files changed, 46 insertions(+)

diff --git a/compat/basename.c b/compat/basename.c
index 0f1b0b0..0a2ed25 100644
--- a/compat/basename.c
+++ b/compat/basename.c

@@ -1,4 +1,5 @@
 #include "../git-compat-util.h"
+#include "../strbuf.h"
 
 /* Adapted from libiberty's basename.c.  */
 char *gitbasename (char *path)

@@ -25,3 +26,46 @@ char *gitbasename (char *path)
 	}
 	return (char *)base;
 }
+
+char *gitdirname(char *path)
+{
+	char *p = path, *slash = NULL, c;
+	int dos_drive_prefix;
+
+	if (!p)
+		return ".";
+
+	if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p) {
+		static struct strbuf buf = STRBUF_INIT;
+
+dot:
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "%.*s.", dos_drive_prefix, path);
+		return buf.buf;
+	}
+
+	/*
+	 * POSIX.1-2001 says dirname("/") should return "/", and dirname("//")
+	 * should return "//", but dirname("///") should return "/" again.
+	 */
+	if (is_dir_sep(*p)) {
+		if (!p[1] || (is_dir_sep(p[1]) && !p[2]))
+			return path;
+		slash = ++p;
+	}
+	while ((c = *(p++)))
+		if (is_dir_sep(c)) {
+			char *tentative = p - 1;
+
+			/* POSIX.1-2001 says to ignore trailing slashes */
+			while (is_dir_sep(*p))
+				p++;
+			if (*p)
+				slash = tentative;
+		}
+
+	if (!slash)
+		goto dot;
+	*slash = '\0';
+	return path;
+}

diff --git a/git-compat-util.h b/git-compat-util.h
index 0d66f3a..94f311a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -253,6 +253,8 @@ struct itimerval {
 #else
 #define basename gitbasename
 extern char *gitbasename(char *);
+#define dirname gitdirname
+extern char *gitdirname(char *);
 #endif
 
 #ifndef NO_ICONV

-- 
2.6.3.windows.1.300.g1c25e49

Unfortunately, some libgen implementations yield outcomes different from
what Git expects. For example, mingw-w64-crt provides a basename()
function, that shortens `path0/` to `path`!

So let's verify that the basename() and dirname() functions we use conform
to what Git expects.

Derived-from-code-by: Ramsay Jones [off-list ref]
Signed-off-by: Johannes Schindelin <redacted>
---
 t/t0060-path-utils.sh |   3 +
 test-path-utils.c     | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 171 insertions(+)

diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 627ef85..f0152a7 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh

@@ -59,6 +59,9 @@ case $(uname -s) in
 	;;
 esac
 
+test_expect_success basename 'test-path-utils basename'
+test_expect_success dirname 'test-path-utils dirname'
+
 norm_path "" ""
 norm_path . ""
 norm_path ./ ""

diff --git a/test-path-utils.c b/test-path-utils.c
index c67bf65..74e74c9 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c

@@ -39,6 +39,168 @@ static void normalize_argv_string(const char **var, const char *input)
 		die("Bad value: %s\n", input);
 }
 
+struct test_data {
+	char *from;  /* input:  transform from this ... */
+	char *to;    /* output: ... to this.            */
+};
+
+static int test_function(struct test_data *data, char *(*func)(char *input),
+	const char *funcname)
+{
+	int failed = 0, i;
+	static char buffer[1024];
+	char *to;
+
+	for (i = 0; data[i].to; i++) {
+		if (!data[i].from)
+			to = func(NULL);
+		else {
+			strcpy(buffer, data[i].from);
+			to = func(buffer);
+		}
+		if (strcmp(to, data[i].to)) {
+			error("FAIL: %s(%s) => '%s' != '%s'\n",
+				funcname, data[i].from, to, data[i].to);
+			failed++;
+		}
+	}
+	return !!failed;
+}
+
+static struct test_data basename_data[] = {
+	/* --- POSIX type paths --- */
+	{ NULL,              "."    },
+	{ "",                "."    },
+	{ ".",               "."    },
+	{ "..",              ".."   },
+	{ "/",               "/"    },
+#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)
+	{ "//",              "//"   },
+	{ "///",             "//"   },
+	{ "////",            "//"   },
+#else
+	{ "//",              "/"    },
+	{ "///",             "/"    },
+	{ "////",            "/"    },
+#endif
+	{ "usr",             "usr"  },
+	{ "/usr",            "usr"  },
+	{ "/usr/",           "usr"  },
+	{ "/usr//",          "usr"  },
+	{ "/usr/lib",        "lib"  },
+	{ "usr/lib",         "lib"  },
+	{ "usr/lib///",      "lib"  },
+
+#if defined(__MINGW32__) || defined(_MSC_VER)
+
+	/* --- win32 type paths --- */
+	{ "\\usr",           "usr"  },
+	{ "\\usr\\",         "usr"  },
+	{ "\\usr\\\\",       "usr"  },
+	{ "\\usr\\lib",      "lib"  },
+	{ "usr\\lib",        "lib"  },
+	{ "usr\\lib\\\\\\",  "lib"  },
+	{ "C:/usr",          "usr"  },
+	{ "C:/usr",          "usr"  },
+	{ "C:/usr/",         "usr"  },
+	{ "C:/usr//",        "usr"  },
+	{ "C:/usr/lib",      "lib"  },
+	{ "C:usr/lib",       "lib"  },
+	{ "C:usr/lib///",    "lib"  },
+	{ "C:",              "."    },
+	{ "C:a",             "a"    },
+	{ "C:/",             "/"    },
+	{ "C:///",           "/"    },
+#if defined(NO_LIBGEN_H)
+	{ "\\",              "\\"   },
+	{ "\\\\",            "\\"   },
+	{ "\\\\\\",          "\\"   },
+#else
+
+	/* win32 platform variations: */
+#if defined(__MINGW32__)
+	{ "\\",              "/"    },
+	{ "\\\\",            "/"    },
+	{ "\\\\\\",          "/"    },
+#endif
+
+#if defined(_MSC_VER)
+	{ "\\",              "\\"   },
+	{ "\\\\",            "\\"   },
+	{ "\\\\\\",          "\\"   },
+#endif
+
+#endif
+#endif
+	{ NULL,              "."    },
+	{ NULL,              NULL   }
+};
+
+static struct test_data dirname_data[] = {
+	/* --- POSIX type paths --- */
+	{ NULL,              "."      },
+	{ "",                "."      },
+	{ ".",               "."      },
+	{ "..",              "."      },
+	{ "/",               "/"      },
+	{ "//",              "//"     },
+#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)
+	{ "///",             "//"     },
+	{ "////",            "//"     },
+#else
+	{ "///",             "/"      },
+	{ "////",            "/"      },
+#endif
+	{ "usr",             "."      },
+	{ "/usr",            "/"      },
+	{ "/usr/",           "/"      },
+	{ "/usr//",          "/"      },
+	{ "/usr/lib",        "/usr"   },
+	{ "usr/lib",         "usr"    },
+	{ "usr/lib///",      "usr"    },
+
+#if defined(__MINGW32__) || defined(_MSC_VER)
+
+	/* --- win32 type paths --- */
+	{ "\\",              "\\"     },
+	{ "\\\\",            "\\\\"   },
+	{ "\\usr",           "\\"     },
+	{ "\\usr\\",         "\\"     },
+	{ "\\usr\\\\",       "\\"     },
+	{ "\\usr\\lib",      "\\usr"  },
+	{ "usr\\lib",        "usr"    },
+	{ "usr\\lib\\\\\\",  "usr"    },
+	{ "C:a",             "C:."    },
+	{ "C:/",             "C:/"    },
+	{ "C:///",           "C:/"    },
+	{ "C:/usr",          "C:/"    },
+	{ "C:/usr/",         "C:/"    },
+	{ "C:/usr//",        "C:/"    },
+	{ "C:/usr/lib",      "C:/usr" },
+	{ "C:usr/lib",       "C:usr"  },
+	{ "C:usr/lib///",    "C:usr"  },
+	{ "\\\\\\",          "\\"     },
+	{ "\\\\\\\\",        "\\"     },
+#if defined(NO_LIBGEN_H)
+	{ "C:",              "C:."    },
+#else
+
+	/* win32 platform variations: */
+#if defined(__MINGW32__)
+	/* the following is clearly wrong ... */
+	{ "C:",              "."      },
+#endif
+
+#if defined(_MSC_VER)
+	{ "C:",              "C:."    },
+#endif
+
+#endif
+#endif
+	{ NULL,              "."      },
+	{ NULL,              NULL     }
+};
+
 int main(int argc, char **argv)
 {
 	if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {

@@ -133,6 +295,12 @@ int main(int argc, char **argv)
 		return 0;
 	}
 
+	if (argc == 2 && !strcmp(argv[1], "basename"))
+		return test_function(basename_data, basename, argv[1]);
+
+	if (argc == 2 && !strcmp(argv[1], "dirname"))
+		return test_function(dirname_data, dirname, argv[1]);
+
 	fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
 		argv[1] ? argv[1] : "(there was none)");
 	return 1;

-- 
2.6.3.windows.1.300.g1c25e49

According to POSIX, basename("/path/") should return "path", not
"path/". Likewise, basename(NULL) and basename("abc") should both
return ".".

Signed-off-by: Johannes Schindelin <redacted>
---
 compat/basename.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/compat/basename.c b/compat/basename.c
index 9f00421..0f1b0b0 100644
--- a/compat/basename.c
+++ b/compat/basename.c

@@ -4,10 +4,24 @@
 char *gitbasename (char *path)
 {
 	const char *base;
-	skip_dos_drive_prefix(&path);
+
+	if (path)
+		skip_dos_drive_prefix(&path);
+
+	if (!path || !*path)
+		return ".";
+
 	for (base = path; *path; path++) {
-		if (is_dir_sep(*path))
-			base = path + 1;
+		if (!is_dir_sep(*path))
+			continue;
+		do {
+			path++;
+		} while (is_dir_sep(*path));
+		if (*path)
+			base = path;
+		else
+			while (--path != base && is_dir_sep(*path))
+				*path = '\0';
 	}
 	return (char *)base;
 }

-- 
2.6.3.windows.1.300.g1c25e49

This mini series adds a fall-back for the `dirname()` function that we use
e.g. in git-am. This is necessary because not all platforms have a working
libgen.h. MSys2 (on which Git for Windows relies) does have a libgen.h, but
its `basename()` implementation is broken, thus we cannot use it.


Johannes Schindelin (4):
  Refactor skipping DOS drive prefixes
  compat/basename: make basename() conform to POSIX
  Provide a dirname() function when NO_LIBGEN_H=YesPlease
  t0060: verify that basename() and dirname() work as expected

 compat/basename.c     |  66 ++++++++++++++++++--
 compat/mingw.c        |  14 ++---
 compat/mingw.h        |  10 ++-
 git-compat-util.h     |  10 +++
 path.c                |  14 ++---
 t/t0060-path-utils.sh |   3 +
 test-path-utils.c     | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 261 insertions(+), 24 deletions(-)

Interdiff vs v1:

 diff --git a/compat/basename.c b/compat/basename.c
 index 10dba38..0a2ed25 100644
 --- a/compat/basename.c
 +++ b/compat/basename.c
 @@ -1,28 +1,58 @@
  #include "../git-compat-util.h"
 +#include "../strbuf.h"
  
  /* Adapted from libiberty's basename.c.  */
  char *gitbasename (char *path)
  {
  	const char *base;
 -	/* Skip over the disk name in MSDOS pathnames. */
 -	if (has_dos_drive_prefix(path))
 -		path += 2;
 +
 +	if (path)
 +		skip_dos_drive_prefix(&path);
 +
 +	if (!path || !*path)
 +		return ".";
 +
  	for (base = path; *path; path++) {
 -		if (is_dir_sep(*path))
 -			base = path + 1;
 +		if (!is_dir_sep(*path))
 +			continue;
 +		do {
 +			path++;
 +		} while (is_dir_sep(*path));
 +		if (*path)
 +			base = path;
 +		else
 +			while (--path != base && is_dir_sep(*path))
 +				*path = '\0';
  	}
  	return (char *)base;
  }
  
  char *gitdirname(char *path)
  {
 -	char *p = path, *slash, c;
 +	char *p = path, *slash = NULL, c;
 +	int dos_drive_prefix;
 +
 +	if (!p)
 +		return ".";
  
 -	/* Skip over the disk name in MSDOS pathnames. */
 -	if (has_dos_drive_prefix(p))
 -		p += 2;
 -	/* POSIX.1-2001 says dirname("/") should return "/" */
 -	slash = is_dir_sep(*p) ? ++p : NULL;
 +	if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p) {
 +		static struct strbuf buf = STRBUF_INIT;
 +
 +dot:
 +		strbuf_reset(&buf);
 +		strbuf_addf(&buf, "%.*s.", dos_drive_prefix, path);
 +		return buf.buf;
 +	}
 +
 +	/*
 +	 * POSIX.1-2001 says dirname("/") should return "/", and dirname("//")
 +	 * should return "//", but dirname("///") should return "/" again.
 +	 */
 +	if (is_dir_sep(*p)) {
 +		if (!p[1] || (is_dir_sep(p[1]) && !p[2]))
 +			return path;
 +		slash = ++p;
 +	}
  	while ((c = *(p++)))
  		if (is_dir_sep(c)) {
  			char *tentative = p - 1;
 @@ -35,7 +65,7 @@ char *gitdirname(char *path)
  		}
  
  	if (!slash)
 -		return ".";
 +		goto dot;
  	*slash = '\0';
  	return path;
  }
 diff --git a/compat/mingw.c b/compat/mingw.c
 index 5edea29..1b3530a 100644
 --- a/compat/mingw.c
 +++ b/compat/mingw.c
 @@ -1934,26 +1934,22 @@ pid_t waitpid(pid_t pid, int *status, int options)
  
  int mingw_offset_1st_component(const char *path)
  {
 -	int offset = 0;
 -	if (has_dos_drive_prefix(path))
 -		offset = 2;
 +	char *pos = (char *)path;
  
  	/* unc paths */
 -	else if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {
 -
 +	if (!skip_dos_drive_prefix(&pos) &&
 +			is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
  		/* skip server name */
 -		char *pos = strpbrk(path + 2, "\\/");
 +		pos = strpbrk(pos + 2, "\\/");
  		if (!pos)
  			return 0; /* Error: malformed unc path */
  
  		do {
  			pos++;
  		} while (*pos && !is_dir_sep(*pos));
 -
 -		offset = pos - path;
  	}
  
 -	return offset + is_dir_sep(path[offset]);
 +	return pos + is_dir_sep(*pos) - path;
  }
  
  int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)
 diff --git a/compat/mingw.h b/compat/mingw.h
 index 57ca477..b3e5044 100644
 --- a/compat/mingw.h
 +++ b/compat/mingw.h
 @@ -361,7 +361,15 @@ HANDLE winansi_get_osfhandle(int fd);
   * git specific compatibility
   */
  
 -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')
 +#define has_dos_drive_prefix(path) \
 +	(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
 +static inline int mingw_skip_dos_drive_prefix(char **path)
 +{
 +	int ret = has_dos_drive_prefix(*path);
 +	*path += ret;
 +	return ret;
 +}
 +#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix
  #define is_dir_sep(c) ((c) == '/' || (c) == '\\')
  static inline char *mingw_find_last_dir_sep(const char *path)
  {
 diff --git a/git-compat-util.h b/git-compat-util.h
 index 996ee17..94f311a 100644
 --- a/git-compat-util.h
 +++ b/git-compat-util.h
 @@ -337,6 +337,14 @@ static inline int git_has_dos_drive_prefix(const char *path)
  #define has_dos_drive_prefix git_has_dos_drive_prefix
  #endif
  
 +#ifndef skip_dos_drive_prefix
 +static inline int git_skip_dos_drive_prefix(const char **path)
 +{
 +	return 0;
 +}
 +#define skip_dos_drive_prefix git_skip_dos_drive_prefix
 +#endif
 +
  #ifndef is_dir_sep
  static inline int git_is_dir_sep(int c)
  {
 diff --git a/path.c b/path.c
 index 3cd155e..8b7e168 100644
 --- a/path.c
 +++ b/path.c
 @@ -782,13 +782,10 @@ const char *relative_path(const char *in, const char *prefix,
  	else if (!prefix_len)
  		return in;
  
 -	if (have_same_root(in, prefix)) {
 +	if (have_same_root(in, prefix))
  		/* bypass dos_drive, for "c:" is identical to "C:" */
 -		if (has_dos_drive_prefix(in)) {
 -			i = 2;
 -			j = 2;
 -		}
 -	} else {
 +		i = j = has_dos_drive_prefix(in);
 +	else {
  		return in;
  	}
  
 @@ -943,11 +940,10 @@ const char *remove_leading_path(const char *in, const char *prefix)
  int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
  {
  	char *dst0;
 +	int i;
  
 -	if (has_dos_drive_prefix(src)) {
 +	for (i = has_dos_drive_prefix(src); i > 0; i--)
  		*dst++ = *src++;
 -		*dst++ = *src++;
 -	}
  	dst0 = dst;
  
  	if (is_dir_sep(*src)) {
 diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
 index 627ef85..f0152a7 100755
 --- a/t/t0060-path-utils.sh
 +++ b/t/t0060-path-utils.sh
 @@ -59,6 +59,9 @@ case $(uname -s) in
  	;;
  esac
  
 +test_expect_success basename 'test-path-utils basename'
 +test_expect_success dirname 'test-path-utils dirname'
 +
  norm_path "" ""
  norm_path . ""
  norm_path ./ ""
 diff --git a/test-path-utils.c b/test-path-utils.c
 index c67bf65..74e74c9 100644
 --- a/test-path-utils.c
 +++ b/test-path-utils.c
 @@ -39,6 +39,168 @@ static void normalize_argv_string(const char **var, const char *input)
  		die("Bad value: %s\n", input);
  }
  
 +struct test_data {
 +	char *from;  /* input:  transform from this ... */
 +	char *to;    /* output: ... to this.            */
 +};
 +
 +static int test_function(struct test_data *data, char *(*func)(char *input),
 +	const char *funcname)
 +{
 +	int failed = 0, i;
 +	static char buffer[1024];
 +	char *to;
 +
 +	for (i = 0; data[i].to; i++) {
 +		if (!data[i].from)
 +			to = func(NULL);
 +		else {
 +			strcpy(buffer, data[i].from);
 +			to = func(buffer);
 +		}
 +		if (strcmp(to, data[i].to)) {
 +			error("FAIL: %s(%s) => '%s' != '%s'\n",
 +				funcname, data[i].from, to, data[i].to);
 +			failed++;
 +		}
 +	}
 +	return !!failed;
 +}
 +
 +static struct test_data basename_data[] = {
 +	/* --- POSIX type paths --- */
 +	{ NULL,              "."    },
 +	{ "",                "."    },
 +	{ ".",               "."    },
 +	{ "..",              ".."   },
 +	{ "/",               "/"    },
 +#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)
 +	{ "//",              "//"   },
 +	{ "///",             "//"   },
 +	{ "////",            "//"   },
 +#else
 +	{ "//",              "/"    },
 +	{ "///",             "/"    },
 +	{ "////",            "/"    },
 +#endif
 +	{ "usr",             "usr"  },
 +	{ "/usr",            "usr"  },
 +	{ "/usr/",           "usr"  },
 +	{ "/usr//",          "usr"  },
 +	{ "/usr/lib",        "lib"  },
 +	{ "usr/lib",         "lib"  },
 +	{ "usr/lib///",      "lib"  },
 +
 +#if defined(__MINGW32__) || defined(_MSC_VER)
 +
 +	/* --- win32 type paths --- */
 +	{ "\\usr",           "usr"  },
 +	{ "\\usr\\",         "usr"  },
 +	{ "\\usr\\\\",       "usr"  },
 +	{ "\\usr\\lib",      "lib"  },
 +	{ "usr\\lib",        "lib"  },
 +	{ "usr\\lib\\\\\\",  "lib"  },
 +	{ "C:/usr",          "usr"  },
 +	{ "C:/usr",          "usr"  },
 +	{ "C:/usr/",         "usr"  },
 +	{ "C:/usr//",        "usr"  },
 +	{ "C:/usr/lib",      "lib"  },
 +	{ "C:usr/lib",       "lib"  },
 +	{ "C:usr/lib///",    "lib"  },
 +	{ "C:",              "."    },
 +	{ "C:a",             "a"    },
 +	{ "C:/",             "/"    },
 +	{ "C:///",           "/"    },
 +#if defined(NO_LIBGEN_H)
 +	{ "\\",              "\\"   },
 +	{ "\\\\",            "\\"   },
 +	{ "\\\\\\",          "\\"   },
 +#else
 +
 +	/* win32 platform variations: */
 +#if defined(__MINGW32__)
 +	{ "\\",              "/"    },
 +	{ "\\\\",            "/"    },
 +	{ "\\\\\\",          "/"    },
 +#endif
 +
 +#if defined(_MSC_VER)
 +	{ "\\",              "\\"   },
 +	{ "\\\\",            "\\"   },
 +	{ "\\\\\\",          "\\"   },
 +#endif
 +
 +#endif
 +#endif
 +	{ NULL,              "."    },
 +	{ NULL,              NULL   }
 +};
 +
 +static struct test_data dirname_data[] = {
 +	/* --- POSIX type paths --- */
 +	{ NULL,              "."      },
 +	{ "",                "."      },
 +	{ ".",               "."      },
 +	{ "..",              "."      },
 +	{ "/",               "/"      },
 +	{ "//",              "//"     },
 +#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)
 +	{ "///",             "//"     },
 +	{ "////",            "//"     },
 +#else
 +	{ "///",             "/"      },
 +	{ "////",            "/"      },
 +#endif
 +	{ "usr",             "."      },
 +	{ "/usr",            "/"      },
 +	{ "/usr/",           "/"      },
 +	{ "/usr//",          "/"      },
 +	{ "/usr/lib",        "/usr"   },
 +	{ "usr/lib",         "usr"    },
 +	{ "usr/lib///",      "usr"    },
 +
 +#if defined(__MINGW32__) || defined(_MSC_VER)
 +
 +	/* --- win32 type paths --- */
 +	{ "\\",              "\\"     },
 +	{ "\\\\",            "\\\\"   },
 +	{ "\\usr",           "\\"     },
 +	{ "\\usr\\",         "\\"     },
 +	{ "\\usr\\\\",       "\\"     },
 +	{ "\\usr\\lib",      "\\usr"  },
 +	{ "usr\\lib",        "usr"    },
 +	{ "usr\\lib\\\\\\",  "usr"    },
 +	{ "C:a",             "C:."    },
 +	{ "C:/",             "C:/"    },
 +	{ "C:///",           "C:/"    },
 +	{ "C:/usr",          "C:/"    },
 +	{ "C:/usr/",         "C:/"    },
 +	{ "C:/usr//",        "C:/"    },
 +	{ "C:/usr/lib",      "C:/usr" },
 +	{ "C:usr/lib",       "C:usr"  },
 +	{ "C:usr/lib///",    "C:usr"  },
 +	{ "\\\\\\",          "\\"     },
 +	{ "\\\\\\\\",        "\\"     },
 +#if defined(NO_LIBGEN_H)
 +	{ "C:",              "C:."    },
 +#else
 +
 +	/* win32 platform variations: */
 +#if defined(__MINGW32__)
 +	/* the following is clearly wrong ... */
 +	{ "C:",              "."      },
 +#endif
 +
 +#if defined(_MSC_VER)
 +	{ "C:",              "C:."    },
 +#endif
 +
 +#endif
 +#endif
 +	{ NULL,              "."      },
 +	{ NULL,              NULL     }
 +};
 +
  int main(int argc, char **argv)
  {
  	if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
 @@ -133,6 +295,12 @@ int main(int argc, char **argv)
  		return 0;
  	}
  
 +	if (argc == 2 && !strcmp(argv[1], "basename"))
 +		return test_function(basename_data, basename, argv[1]);
 +
 +	if (argc == 2 && !strcmp(argv[1], "dirname"))
 +		return test_function(dirname_data, dirname, argv[1]);
 +
  	fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
  		argv[1] ? argv[1] : "(there was none)");
  	return 1;

-- 
2.6.3.windows.1.300.g1c25e49

On Fri, Jan 8, 2016 at 11:21 AM, Johannes Schindelin
[off-list ref] wrote:

Junio Hamano pointed out that there is an implicit assumption in pretty
much all the code calling has_dos_drive_prefix(): it assumes that the
DOS drive prefix is always two bytes long.

While this assumption is pretty safe, we can still make the code more
readable and less error-prone by introducing a function that skips the
DOS drive prefix safely.

While at it, we change the has_dos_drive_prefix() return value: it now
returns the number of bytes to be skipped if there is a DOS drive prefix.

With this change, code such as:

    for (i = has_dos_drive_prefix(src); i > 0; i--)
        ...

in path.c reads a bit oddly. Renaming the function might help. For instance:

    for (i = dos_drive_prefix_len(src); i > 0; i--)
        ...

Signed-off-by: Johannes Schindelin <redacted>

On Fri, Jan 8, 2016 at 11:21 AM, Johannes Schindelin
[off-list ref] wrote:

Can these be 'const'? If I'm reading the code correctly, I don't think
these values ever get passed directly to functions expecting non-const
strings.

+};
+
+static int test_function(struct test_data *data, char *(*func)(char *input),
+       const char *funcname)
+{
+       int failed = 0, i;
+       static char buffer[1024];

Why is this 'static'? It is never accessed outside of this scope.

+       char *to;
+
+       for (i = 0; data[i].to; i++) {
+               if (!data[i].from)
+                       to = func(NULL);
+               else {
+                       strcpy(buffer, data[i].from);
+                       to = func(buffer);
+               }
+               if (strcmp(to, data[i].to)) {
+                       error("FAIL: %s(%s) => '%s' != '%s'\n",
+                               funcname, data[i].from, to, data[i].to);
+                       failed++;

Since 'failed' is only ever used as a boolean, it might be clearer to say:

    failed = 1;

+               }
+       }
+       return !!failed;

And then simply:

    return failed;

+}
+
+static struct test_data basename_data[] = {
+       /* --- POSIX type paths --- */
+       { NULL,              "."    },

NULL is tested here.

+       { "",                "."    },
+       { ".",               "."    },
[...]
+#endif
+       { NULL,              "."    },

And also here. Is that intentional?

+       { NULL,              NULL   }
+};
+
+static struct test_data dirname_data[] = {
+       /* --- POSIX type paths --- */
+       { NULL,              "."      },
[...]
+#endif
+       { NULL,              "."      },

Ditto.

+       { NULL,              NULL     }
+};

Hi Eric,

On Sat, 9 Jan 2016, Eric Sunshine wrote:

On Fri, Jan 8, 2016 at 11:21 AM, Johannes Schindelin
[off-list ref] wrote:

quoted

Unfortunately, some libgen implementations yield outcomes different from
what Git expects. For example, mingw-w64-crt provides a basename()
function, that shortens `path0/` to `path`!

So let's verify that the basename() and dirname() functions we use conform
to what Git expects.

Derived-from-code-by: Ramsay Jones [off-list ref]
Signed-off-by: Johannes Schindelin <redacted>
---

diff --git a/test-path-utils.c b/test-path-utils.c

@@ -39,6 +39,168 @@ static void normalize_argv_string(const char **var, const char *input)
+struct test_data {
+       char *from;  /* input:  transform from this ... */
+       char *to;    /* output: ... to this.            */

Can these be 'const'? If I'm reading the code correctly, I don't think
these values ever get passed directly to functions expecting non-const
strings.

This, and ...

quoted

+};
+
+static int test_function(struct test_data *data, char *(*func)(char *input),
+       const char *funcname)
+{
+       int failed = 0, i;
+       static char buffer[1024];

Why is this 'static'? It is never accessed outside of this scope.

... this, and ...

quoted

+       char *to;
+
+       for (i = 0; data[i].to; i++) {
+               if (!data[i].from)
+                       to = func(NULL);
+               else {
+                       strcpy(buffer, data[i].from);
+                       to = func(buffer);
+               }
+               if (strcmp(to, data[i].to)) {
+                       error("FAIL: %s(%s) => '%s' != '%s'\n",
+                               funcname, data[i].from, to, data[i].to);
+                       failed++;

Since 'failed' is only ever used as a boolean, it might be clearer to say:

    failed = 1;

... this and ...

quoted

+               }
+       }
+       return !!failed;

And then simply:

    return failed;

quoted

+}
+
+static struct test_data basename_data[] = {
+       /* --- POSIX type paths --- */
+       { NULL,              "."    },

NULL is tested here.

quoted

+       { "",                "."    },
+       { ".",               "."    },
[...]
+#endif
+       { NULL,              "."    },

And also here. Is that intentional?

... this are all valid concerns that I now addressed locally, so they will
be fixed in the next iteration.

Thanks,
Dscho

Junio Hamano pointed out that there is an implicit assumption in pretty
much all the code calling has_dos_drive_prefix(): it assumes that the
DOS drive prefix is always two bytes long.

While this assumption is pretty safe, we can still make the code more
readable and less error-prone by introducing a function that skips the
DOS drive prefix safely.

While at it, we change the has_dos_drive_prefix() return value: it now
returns the number of bytes to be skipped if there is a DOS drive prefix.

Signed-off-by: Johannes Schindelin <redacted>
---
 compat/basename.c |  4 +---
 compat/mingw.c    | 14 +++++---------
 compat/mingw.h    | 10 +++++++++-
 git-compat-util.h |  8 ++++++++
 path.c            | 14 +++++---------
 5 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/compat/basename.c b/compat/basename.c
index d8f8a3c..9f00421 100644
--- a/compat/basename.c
+++ b/compat/basename.c

@@ -4,9 +4,7 @@
 char *gitbasename (char *path)
 {
 	const char *base;
-	/* Skip over the disk name in MSDOS pathnames. */
-	if (has_dos_drive_prefix(path))
-		path += 2;
+	skip_dos_drive_prefix(&path);
 	for (base = path; *path; path++) {
 		if (is_dir_sep(*path))
 			base = path + 1;

diff --git a/compat/mingw.c b/compat/mingw.c
index 5edea29..1b3530a 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c

@@ -1934,26 +1934,22 @@ pid_t waitpid(pid_t pid, int *status, int options)
 
 int mingw_offset_1st_component(const char *path)
 {
-	int offset = 0;
-	if (has_dos_drive_prefix(path))
-		offset = 2;
+	char *pos = (char *)path;
 
 	/* unc paths */
-	else if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {
-
+	if (!skip_dos_drive_prefix(&pos) &&
+			is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
 		/* skip server name */
-		char *pos = strpbrk(path + 2, "\\/");
+		pos = strpbrk(pos + 2, "\\/");
 		if (!pos)
 			return 0; /* Error: malformed unc path */
 
 		do {
 			pos++;
 		} while (*pos && !is_dir_sep(*pos));
-
-		offset = pos - path;
 	}
 
-	return offset + is_dir_sep(path[offset]);
+	return pos + is_dir_sep(*pos) - path;
 }
 
 int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)

diff --git a/compat/mingw.h b/compat/mingw.h
index 57ca477..b3e5044 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h

@@ -361,7 +361,15 @@ HANDLE winansi_get_osfhandle(int fd);
  * git specific compatibility
  */
 
-#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')
+#define has_dos_drive_prefix(path) \
+	(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
+static inline int mingw_skip_dos_drive_prefix(char **path)
+{
+	int ret = has_dos_drive_prefix(*path);
+	*path += ret;
+	return ret;
+}
+#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix
 #define is_dir_sep(c) ((c) == '/' || (c) == '\\')
 static inline char *mingw_find_last_dir_sep(const char *path)
 {

diff --git a/git-compat-util.h b/git-compat-util.h
index 2da0a75..0d66f3a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -335,6 +335,14 @@ static inline int git_has_dos_drive_prefix(const char *path)
 #define has_dos_drive_prefix git_has_dos_drive_prefix
 #endif
 
+#ifndef skip_dos_drive_prefix
+static inline int git_skip_dos_drive_prefix(const char **path)
+{
+	return 0;
+}
+#define skip_dos_drive_prefix git_skip_dos_drive_prefix
+#endif
+
 #ifndef is_dir_sep
 static inline int git_is_dir_sep(int c)
 {

diff --git a/path.c b/path.c
index 3cd155e..8b7e168 100644
--- a/path.c
+++ b/path.c

@@ -782,13 +782,10 @@ const char *relative_path(const char *in, const char *prefix,
 	else if (!prefix_len)
 		return in;
 
-	if (have_same_root(in, prefix)) {
+	if (have_same_root(in, prefix))
 		/* bypass dos_drive, for "c:" is identical to "C:" */
-		if (has_dos_drive_prefix(in)) {
-			i = 2;
-			j = 2;
-		}
-	} else {
+		i = j = has_dos_drive_prefix(in);
+	else {
 		return in;
 	}
 

@@ -943,11 +940,10 @@ const char *remove_leading_path(const char *in, const char *prefix)
 int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
 {
 	char *dst0;
+	int i;
 
-	if (has_dos_drive_prefix(src)) {
+	for (i = has_dos_drive_prefix(src); i > 0; i--)
 		*dst++ = *src++;
-		*dst++ = *src++;
-	}
 	dst0 = dst;
 
 	if (is_dir_sep(*src)) {

-- 
2.6.3.windows.1.300.g1c25e49

According to POSIX, basename("/path/") should return "path", not
"path/". Likewise, basename(NULL) and basename("") should both
return "." to conform.

Signed-off-by: Johannes Schindelin <redacted>
---
 compat/basename.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/compat/basename.c b/compat/basename.c
index 9f00421..0f1b0b0 100644
--- a/compat/basename.c
+++ b/compat/basename.c

@@ -4,10 +4,24 @@
 char *gitbasename (char *path)
 {
 	const char *base;
-	skip_dos_drive_prefix(&path);
+
+	if (path)
+		skip_dos_drive_prefix(&path);
+
+	if (!path || !*path)
+		return ".";
+
 	for (base = path; *path; path++) {
-		if (is_dir_sep(*path))
-			base = path + 1;
+		if (!is_dir_sep(*path))
+			continue;
+		do {
+			path++;
+		} while (is_dir_sep(*path));
+		if (*path)
+			base = path;
+		else
+			while (--path != base && is_dir_sep(*path))
+				*path = '\0';
 	}
 	return (char *)base;
 }

-- 
2.6.3.windows.1.300.g1c25e49

This mini series adds a fall-back for the `dirname()` function that we use
e.g. in git-am. This is necessary because not all platforms have a working
libgen.h.

While at it, we ensure that our basename() drop-in conforms to the POSIX
specifications.

In addition to the interdiff vs v2, the commit message was fixed to
mention basename("") as cornercase (not basename("abc")).


Johannes Schindelin (4):
  Refactor skipping DOS drive prefixes
  compat/basename: make basename() conform to POSIX
  Provide a dirname() function when NO_LIBGEN_H=YesPlease
  t0060: verify that basename() and dirname() work as expected

 compat/basename.c     |  66 ++++++++++++++++++--
 compat/mingw.c        |  14 ++---
 compat/mingw.h        |  10 ++-
 git-compat-util.h     |  10 +++
 path.c                |  14 ++---
 t/t0060-path-utils.sh |   3 +
 test-path-utils.c     | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 259 insertions(+), 24 deletions(-)

Interdiff vs v2:

 diff --git a/test-path-utils.c b/test-path-utils.c
 index 74e74c9..4ab68ac 100644
 --- a/test-path-utils.c
 +++ b/test-path-utils.c
 @@ -40,15 +40,15 @@ static void normalize_argv_string(const char **var, const char *input)
  }
  
  struct test_data {
 -	char *from;  /* input:  transform from this ... */
 -	char *to;    /* output: ... to this.            */
 +	const char *from;  /* input:  transform from this ... */
 +	const char *to;    /* output: ... to this.            */
  };
  
  static int test_function(struct test_data *data, char *(*func)(char *input),
  	const char *funcname)
  {
  	int failed = 0, i;
 -	static char buffer[1024];
 +	char buffer[1024];
  	char *to;
  
  	for (i = 0; data[i].to; i++) {
 @@ -61,10 +61,10 @@ static int test_function(struct test_data *data, char *(*func)(char *input),
  		if (strcmp(to, data[i].to)) {
  			error("FAIL: %s(%s) => '%s' != '%s'\n",
  				funcname, data[i].from, to, data[i].to);
 -			failed++;
 +			failed = 1;
  		}
  	}
 -	return !!failed;
 +	return failed;
  }
  
  static struct test_data basename_data[] = {
 @@ -132,7 +132,6 @@ static struct test_data basename_data[] = {
  
  #endif
  #endif
 -	{ NULL,              "."    },
  	{ NULL,              NULL   }
  };
  
 @@ -197,7 +196,6 @@ static struct test_data dirname_data[] = {
  
  #endif
  #endif
 -	{ NULL,              "."      },
  	{ NULL,              NULL     }
  };
  

-- 
2.6.3.windows.1.300.g1c25e49

Unfortunately, some libgen implementations yield outcomes different
from what Git expects. For example, mingw-w64-crt provides a basename()
function, that shortens `path0/` to `path`!

So let's verify that the basename() and dirname() functions we use
conform to what Git expects.

Derived-from-code-by: Ramsay Jones [off-list ref]
Signed-off-by: Johannes Schindelin <redacted>
---
 t/t0060-path-utils.sh |   3 +
 test-path-utils.c     | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 169 insertions(+)

diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 627ef85..f0152a7 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh

@@ -59,6 +59,9 @@ case $(uname -s) in
 	;;
 esac
 
+test_expect_success basename 'test-path-utils basename'
+test_expect_success dirname 'test-path-utils dirname'
+
 norm_path "" ""
 norm_path . ""
 norm_path ./ ""

diff --git a/test-path-utils.c b/test-path-utils.c
index c67bf65..4ab68ac 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c

@@ -39,6 +39,166 @@ static void normalize_argv_string(const char **var, const char *input)
 		die("Bad value: %s\n", input);
 }
 
+struct test_data {
+	const char *from;  /* input:  transform from this ... */
+	const char *to;    /* output: ... to this.            */
+};
+
+static int test_function(struct test_data *data, char *(*func)(char *input),
+	const char *funcname)
+{
+	int failed = 0, i;
+	char buffer[1024];
+	char *to;
+
+	for (i = 0; data[i].to; i++) {
+		if (!data[i].from)
+			to = func(NULL);
+		else {
+			strcpy(buffer, data[i].from);
+			to = func(buffer);
+		}
+		if (strcmp(to, data[i].to)) {
+			error("FAIL: %s(%s) => '%s' != '%s'\n",
+				funcname, data[i].from, to, data[i].to);
+			failed = 1;
+		}
+	}
+	return failed;
+}
+
+static struct test_data basename_data[] = {
+	/* --- POSIX type paths --- */
+	{ NULL,              "."    },
+	{ "",                "."    },
+	{ ".",               "."    },
+	{ "..",              ".."   },
+	{ "/",               "/"    },
+#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)
+	{ "//",              "//"   },
+	{ "///",             "//"   },
+	{ "////",            "//"   },
+#else
+	{ "//",              "/"    },
+	{ "///",             "/"    },
+	{ "////",            "/"    },
+#endif
+	{ "usr",             "usr"  },
+	{ "/usr",            "usr"  },
+	{ "/usr/",           "usr"  },
+	{ "/usr//",          "usr"  },
+	{ "/usr/lib",        "lib"  },
+	{ "usr/lib",         "lib"  },
+	{ "usr/lib///",      "lib"  },
+
+#if defined(__MINGW32__) || defined(_MSC_VER)
+
+	/* --- win32 type paths --- */
+	{ "\\usr",           "usr"  },
+	{ "\\usr\\",         "usr"  },
+	{ "\\usr\\\\",       "usr"  },
+	{ "\\usr\\lib",      "lib"  },
+	{ "usr\\lib",        "lib"  },
+	{ "usr\\lib\\\\\\",  "lib"  },
+	{ "C:/usr",          "usr"  },
+	{ "C:/usr",          "usr"  },
+	{ "C:/usr/",         "usr"  },
+	{ "C:/usr//",        "usr"  },
+	{ "C:/usr/lib",      "lib"  },
+	{ "C:usr/lib",       "lib"  },
+	{ "C:usr/lib///",    "lib"  },
+	{ "C:",              "."    },
+	{ "C:a",             "a"    },
+	{ "C:/",             "/"    },
+	{ "C:///",           "/"    },
+#if defined(NO_LIBGEN_H)
+	{ "\\",              "\\"   },
+	{ "\\\\",            "\\"   },
+	{ "\\\\\\",          "\\"   },
+#else
+
+	/* win32 platform variations: */
+#if defined(__MINGW32__)
+	{ "\\",              "/"    },
+	{ "\\\\",            "/"    },
+	{ "\\\\\\",          "/"    },
+#endif
+
+#if defined(_MSC_VER)
+	{ "\\",              "\\"   },
+	{ "\\\\",            "\\"   },
+	{ "\\\\\\",          "\\"   },
+#endif
+
+#endif
+#endif
+	{ NULL,              NULL   }
+};
+
+static struct test_data dirname_data[] = {
+	/* --- POSIX type paths --- */
+	{ NULL,              "."      },
+	{ "",                "."      },
+	{ ".",               "."      },
+	{ "..",              "."      },
+	{ "/",               "/"      },
+	{ "//",              "//"     },
+#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)
+	{ "///",             "//"     },
+	{ "////",            "//"     },
+#else
+	{ "///",             "/"      },
+	{ "////",            "/"      },
+#endif
+	{ "usr",             "."      },
+	{ "/usr",            "/"      },
+	{ "/usr/",           "/"      },
+	{ "/usr//",          "/"      },
+	{ "/usr/lib",        "/usr"   },
+	{ "usr/lib",         "usr"    },
+	{ "usr/lib///",      "usr"    },
+
+#if defined(__MINGW32__) || defined(_MSC_VER)
+
+	/* --- win32 type paths --- */
+	{ "\\",              "\\"     },
+	{ "\\\\",            "\\\\"   },
+	{ "\\usr",           "\\"     },
+	{ "\\usr\\",         "\\"     },
+	{ "\\usr\\\\",       "\\"     },
+	{ "\\usr\\lib",      "\\usr"  },
+	{ "usr\\lib",        "usr"    },
+	{ "usr\\lib\\\\\\",  "usr"    },
+	{ "C:a",             "C:."    },
+	{ "C:/",             "C:/"    },
+	{ "C:///",           "C:/"    },
+	{ "C:/usr",          "C:/"    },
+	{ "C:/usr/",         "C:/"    },
+	{ "C:/usr//",        "C:/"    },
+	{ "C:/usr/lib",      "C:/usr" },
+	{ "C:usr/lib",       "C:usr"  },
+	{ "C:usr/lib///",    "C:usr"  },
+	{ "\\\\\\",          "\\"     },
+	{ "\\\\\\\\",        "\\"     },
+#if defined(NO_LIBGEN_H)
+	{ "C:",              "C:."    },
+#else
+
+	/* win32 platform variations: */
+#if defined(__MINGW32__)
+	/* the following is clearly wrong ... */
+	{ "C:",              "."      },
+#endif
+
+#if defined(_MSC_VER)
+	{ "C:",              "C:."    },
+#endif
+
+#endif
+#endif
+	{ NULL,              NULL     }
+};
+
 int main(int argc, char **argv)
 {
 	if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {

@@ -133,6 +293,12 @@ int main(int argc, char **argv)
 		return 0;
 	}
 
+	if (argc == 2 && !strcmp(argv[1], "basename"))
+		return test_function(basename_data, basename, argv[1]);
+
+	if (argc == 2 && !strcmp(argv[1], "dirname"))
+		return test_function(dirname_data, dirname, argv[1]);
+
 	fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
 		argv[1] ? argv[1] : "(there was none)");
 	return 1;

-- 
2.6.3.windows.1.300.g1c25e49

When there is no `libgen.h` to our disposal, we miss the `dirname()`
function.

So far, we only had one user of that function: credential-cache--daemon
(which was only compiled when Unix sockets are available, anyway). But
now we also have `builtin/am.c` as user, so we need it.

Since `dirname()` is a sibling of `basename()`, we simply put our very
own `gitdirname()` implementation next to `gitbasename()` and use it
if `NO_LIBGEN_H` has been set.

Signed-off-by: Johannes Schindelin <redacted>
---
 compat/basename.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 git-compat-util.h |  2 ++
 2 files changed, 46 insertions(+)

diff --git a/compat/basename.c b/compat/basename.c
index 0f1b0b0..0a2ed25 100644
--- a/compat/basename.c
+++ b/compat/basename.c

@@ -1,4 +1,5 @@
 #include "../git-compat-util.h"
+#include "../strbuf.h"
 
 /* Adapted from libiberty's basename.c.  */
 char *gitbasename (char *path)

@@ -25,3 +26,46 @@ char *gitbasename (char *path)
 	}
 	return (char *)base;
 }
+
+char *gitdirname(char *path)
+{
+	char *p = path, *slash = NULL, c;
+	int dos_drive_prefix;
+
+	if (!p)
+		return ".";
+
+	if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p) {
+		static struct strbuf buf = STRBUF_INIT;
+
+dot:
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "%.*s.", dos_drive_prefix, path);
+		return buf.buf;
+	}
+
+	/*
+	 * POSIX.1-2001 says dirname("/") should return "/", and dirname("//")
+	 * should return "//", but dirname("///") should return "/" again.
+	 */
+	if (is_dir_sep(*p)) {
+		if (!p[1] || (is_dir_sep(p[1]) && !p[2]))
+			return path;
+		slash = ++p;
+	}
+	while ((c = *(p++)))
+		if (is_dir_sep(c)) {
+			char *tentative = p - 1;
+
+			/* POSIX.1-2001 says to ignore trailing slashes */
+			while (is_dir_sep(*p))
+				p++;
+			if (*p)
+				slash = tentative;
+		}
+
+	if (!slash)
+		goto dot;
+	*slash = '\0';
+	return path;
+}

diff --git a/git-compat-util.h b/git-compat-util.h
index 0d66f3a..94f311a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -253,6 +253,8 @@ struct itimerval {
 #else
 #define basename gitbasename
 extern char *gitbasename(char *);
+#define dirname gitdirname
+extern char *gitdirname(char *);
 #endif
 
 #ifndef NO_ICONV

-- 
2.6.3.windows.1.300.g1c25e49

On Mon, Jan 11, 2016 at 1:30 PM, Johannes Schindelin
[off-list ref] wrote:

I wonder if this would be a bit easier to follow if it was structured
something like this:

    static struct strbuf buf = STRBUF_INIT;

    if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p)
        goto dot;

    ...
    if (is_dir_sep(*p)) {
        ...
    }
    ...
    while ((c = *(p++)))
        ...

    if (slash) {
        *slash = '\0';
        return path;
    }

    dot:
    strbuf_reset(&buf);
    strbuf_addf(&buf, "%.*s.", dos_drive_prefix, path);
    return buf.buf;

Junio Hamano pointed out that there is an implicit assumption in pretty
much all the code calling has_dos_drive_prefix(): it assumes that the
DOS drive prefix is always two bytes long.

While this assumption is pretty safe, we can still make the code more
readable and less error-prone by introducing a function that skips the
DOS drive prefix safely.

While at it, we change the has_dos_drive_prefix() return value: it now
returns the number of bytes to be skipped if there is a DOS drive prefix.

Signed-off-by: Johannes Schindelin <redacted>
---
 compat/basename.c |  4 +---
 compat/mingw.c    | 14 +++++---------
 compat/mingw.h    | 10 +++++++++-
 git-compat-util.h |  8 ++++++++
 path.c            | 14 +++++---------
 5 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/compat/basename.c b/compat/basename.c
index d8f8a3c..9f00421 100644
--- a/compat/basename.c
+++ b/compat/basename.c

@@ -4,9 +4,7 @@
 char *gitbasename (char *path)
 {
 	const char *base;
-	/* Skip over the disk name in MSDOS pathnames. */
-	if (has_dos_drive_prefix(path))
-		path += 2;
+	skip_dos_drive_prefix(&path);
 	for (base = path; *path; path++) {
 		if (is_dir_sep(*path))
 			base = path + 1;

diff --git a/compat/mingw.c b/compat/mingw.c
index 5edea29..1b3530a 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c

@@ -1934,26 +1934,22 @@ pid_t waitpid(pid_t pid, int *status, int options)
 
 int mingw_offset_1st_component(const char *path)
 {
-	int offset = 0;
-	if (has_dos_drive_prefix(path))
-		offset = 2;
+	char *pos = (char *)path;
 
 	/* unc paths */
-	else if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {
-
+	if (!skip_dos_drive_prefix(&pos) &&
+			is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
 		/* skip server name */
-		char *pos = strpbrk(path + 2, "\\/");
+		pos = strpbrk(pos + 2, "\\/");
 		if (!pos)
 			return 0; /* Error: malformed unc path */
 
 		do {
 			pos++;
 		} while (*pos && !is_dir_sep(*pos));
-
-		offset = pos - path;
 	}
 
-	return offset + is_dir_sep(path[offset]);
+	return pos + is_dir_sep(*pos) - path;
 }
 
 int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)