Re: [PATCH v4 2/4] compat/basename: make basename() conform to POSIX

Hi Ramsay,

On Wed, 13 Jan 2016, Ramsay Jones wrote:

On 12/01/16 07:57, Johannes Schindelin wrote:

quoted

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;
 }

I don't suppose it makes much difference, but I find my version
slightly easier to read:

Yours is better documented, yes, but as I said, I started from what Git
already had and tried to provide as minimal changes as possible, to make
reviewing easy. In any case, I am very reluctant when it comes to
wholesale code replacements: in my experience, these frequently lead to
new, entertaining and unintended behavior. I worked with somebody who (for
the sake of charity) in the following I will reference only by his most
frequent commit message: Dr "Completely new version" (and yes, this was
the extent of the commit message). If you buy me a beer or three, I will
gladly tell you all the fun I had trying to find the regressions in that
code.

In short: please accept that my decision to build on the existing code
rather than replacing it had nothing to do with your code.

Ciao,
Dscho