Re: [PATCH v4 0/4] Ensure that we can build without libgen.h

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

Hi Johannes,

Sorry for not commenting sooner, I've been away from email for
a few days. Also, I have only just looked at what is currently
in pu (@1a05310), which I'm pretty sure is v3 of this series.

On 12/01/16 07:57, Johannes Schindelin wrote:
> 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.

I was somewhat disappointed that you ignored the implementation of
gitbasename() and gitdirname() that was included in the test-libgen.c
file that I sent you. I had hoped they would be (at worst) a good starting
point if you found them to be lacking for your use case (ie. for the
64-bit versions of MSVC/MinGW).

Did you have any test cases that failed? (If so, could you please add
them to the tests).

Hmm, I just had another look at them and recalled one of my TODO items.
Ahem, yes, ... err, replace code which provoked undefined behaviour. :-P

Actually, that took just ten minutes to fix. (patch below)

> 
> In addition to Eric's style improvement, v4 also fixes the signature
> of skip_dos_drive_prefix() in the non-Windows case.

Yes, this fixes one of my comments about v3.

ATB,
Ramsay Jones
-- >8 --
From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Tue, 12 Jan 2016 23:28:09 +0000
Subject: [PATCH] test-libgen.c: don't provoke undefined behaviour

---
 test-libgen.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/test-libgen.c b/test-libgen.c
index aa3bd18..3024bf1 100644
--- a/test-libgen.c
+++ b/test-libgen.c
@@ -42,9 +42,11 @@ char *gitbasename (char *path)
 		*p-- = '\0';
 	}
 	/* find begining of last path component */
-	while (p >= path && !is_dir_sep(*p))
+	while (p > path && !is_dir_sep(*p))
 		p--;
-	return p + 1;
+	if (is_dir_sep(*p))
+		p++;
+	return p;
 }
 
 char *gitdirname(char *path)
@@ -71,13 +73,12 @@ char *gitdirname(char *path)
 		*p-- = '\0';
 	}
 	/* find begining of last path component */
-	while (p >= start && !is_dir_sep(*p))
+	while (p > start && !is_dir_sep(*p))
 		p--;
 	/* terminate dirname */
-	if (p < start) {
-		p = start;
+	if (p == start && !is_dir_sep(*p))
 		*p++ = '.';
-	} else if (p == start)
+	else if (p == start)
 		p++;
 	*p = '\0';
 	return path;
-- 
2.7.0