{"thread":{"id":"40458","subject":"[PATCH] Provide a dirname() function when NO_LIBGEN_H=YesPlease","startedAt":"2015-09-30T14:50:34Z","lastAt":"2016-01-25T22:04:32Z","messageCount":63,"participants":["Johannes Schindelin","Junio C Hamano","Ramsay Jones","Eric Sunshine","Torsten Bögershausen","Michael Blume","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"270934","messageId":"25a2598e756959f55f06ae6b4dc6f448e3b6b127.1443624188.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":null,"subject":"[PATCH] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-09-30T14:50:34Z","receivedAt":"2015-09-30T14:50:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When there is no `libgen.h` to our disposal, we miss the `dirname()`\nfunction.\n\nSo far, we only had one user of that function: credential-cache--daemon\n(which was only compiled when Unix sockets are available, anyway). But\nnow we also have `builtin/am.c` as user, so we need it.\n\nSince `dirname()` is a sibling of `basename()`, we simply put our very\nown `gitdirname()` implementation next to `gitbasename()` and use it\nif `NO_LIBGEN_H` has been set.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tI stumbled over the compile warning when upgrading Git for Windows\n\tto 2.6.0. There was a left-over NO_LIBGEN_H=YesPlease (which we\n\tno longer need in Git for Windows 2.x), but it did point to the\n\tfact that we use `dirname()` in builtin/am.c now, so we better\n\thave a fall-back implementation for platforms without libgen.h.\n\n\tI tested this implementation a bit, but I still would appreciate\n\ta few eye-balls to go over it.\n\n compat/basename.c | 26 ++++++++++++++++++++++++++\n git-compat-util.h | 2 ++\n 2 files changed, 28 insertions(+)\n\ndiff --git a/compat/basename.c b/compat/basename.c\nindex d8f8a3c..10dba38 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -13,3 +13,29 @@ char *gitbasename (char *path)\n \t}\n \treturn (char *)base;\n }\n+\n+char *gitdirname(char *path)\n+{\n+\tchar *p = path, *slash, c;\n+\n+\t/* Skip over the disk name in MSDOS pathnames. */\n+\tif (has_dos_drive_prefix(p))\n+\t\tp += 2;\n+\t/* POSIX.1-2001 says dirname(\"/\") should return \"/\" */\n+\tslash = is_dir_sep(*p) ? ++p : NULL;\n+\twhile ((c = *(p++)))\n+\t\tif (is_dir_sep(c)) {\n+\t\t\tchar *tentative = p - 1;\n+\n+\t\t\t/* POSIX.1-2001 says to ignore trailing slashes */\n+\t\t\twhile (is_dir_sep(*p))\n+\t\t\t\tp++;\n+\t\t\tif (*p)\n+\t\t\t\tslash = tentative;\n+\t\t}\n+\n+\tif (!slash)\n+\t\treturn \".\";\n+\t*slash = '\\0';\n+\treturn path;\n+}\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex f649e81..8b01aa5 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -253,6 +253,8 @@ struct itimerval {\n #else\n #define basename gitbasename\n extern char *gitbasename(char *);\n+#define dirname gitdirname\n+extern char *gitdirname(char *);\n #endif\n \n #ifndef NO_ICONV\n-- \n2.5.3.windows.1.3.gc322723\n"},{"id":"270937","messageId":"xmqq8u7n934i.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"25a2598e756959f55f06ae6b4dc6f448e3b6b127.1443624188.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-30T18:24:45Z","receivedAt":"2015-09-30T18:24:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> \tI stumbled over the compile warning when upgrading Git for Windows\n> \tto 2.6.0. There was a left-over NO_LIBGEN_H=YesPlease (which we\n> \tno longer need in Git for Windows 2.x), but it did point to the\n> \tfact that we use `dirname()` in builtin/am.c now, so we better\n> \thave a fall-back implementation for platforms without libgen.h.\n\nThanks for being careful.\n\n>\n> \tI tested this implementation a bit, but I still would appreciate\n> \ta few eye-balls to go over it.\n>\n> compat/basename.c | 26 ++++++++++++++++++++++++++\n> git-compat-util.h | 2 ++\n> 2 files changed, 28 insertions(+)\n>\n> diff --git a/compat/basename.c b/compat/basename.c\n> index d8f8a3c..10dba38 100644\n> --- a/compat/basename.c\n> +++ b/compat/basename.c\n> @@ -13,3 +13,29 @@ char *gitbasename (char *path)\n> \t}\n> \treturn (char *)base;\n> }\n> +\n> +char *gitdirname(char *path)\n> +{\n> +\tchar *p = path, *slash, c;\n> +\n> +\t/* Skip over the disk name in MSDOS pathnames. */\n> +\tif (has_dos_drive_prefix(p))\n> +\t\tp += 2;\n\nNot a new problem, but many callers of has_dos_drive_prefix()\nhardcodes that \"2\" in various forms. I wonder if this is something\nwe should relieve callers of by tweaking the semantics of it, e.g.\nby returning 2 (or howmanyever bytes should be skipped) from the\nfunction, changing it to skip_dos_drive_prefix(&p), etc.\n\n> +\t/* POSIX.1-2001 says dirname(\"/\") should return \"/\" */\n> +\tslash = is_dir_sep(*p) ? ++p : NULL;\n> +\twhile ((c = *(p++)))\n\nI am confused by this. What is the invariant on 'p' at the\nbeginning of the body of this while loop in each iteration?\n\nInside the body, p skips over dir-sep characters, so p must point at\nthe byte past the last run of slashes?\n\nIf that is the invariant, upon entry, shouldn't the initialization\nof \"slash\" be skipping over all slashes, not just the first one,\nwhen the input is \"///foo\", for example? Instead the above skips '/'\nand sets slash to the byte past the first '/' (which is OK because\nyou want to NUL-terminate to remove \"//foo\" from the input) but does\nnot move p to 'f', so the invariant is not \"p must point at the byte\npast the last run of slashes\".\n\n> +\t\tif (is_dir_sep(c)) {\n> +\t\t\tchar *tentative = p - 1;\n> +\n> +\t\t\t/* POSIX.1-2001 says to ignore trailing slashes */\n> +\t\t\twhile (is_dir_sep(*p))\n> +\t\t\t\tp++;\n> +\t\t\tif (*p)\n> +\t\t\t\tslash = tentative;\n> +\t\t}\n\nI would have expected the function to scan from the end/right/tail.\n\n> +\tif (!slash)\n> +\t\treturn \".\";\n> +\t*slash = '\\0';\n> +\treturn path;\n> +}\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index f649e81..8b01aa5 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -253,6 +253,8 @@ struct itimerval {\n> #else\n> #define basename gitbasename\n> extern char *gitbasename(char *);\n> +#define dirname gitdirname\n> +extern char *gitdirname(char *);\n> #endif\n> \n> #ifndef NO_ICONV\n"},{"id":"270942","messageId":"560C30B1.3010508@ramsayjones.plus.com","threadId":"40458","inReplyTo":"25a2598e756959f55f06ae6b4dc6f448e3b6b127.1443624188.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2015-09-30T18:57:53Z","receivedAt":"2015-09-30T18:57:53Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Hi Johannes,\n\nOn 30/09/15 15:50, Johannes Schindelin wrote:\n> When there is no `libgen.h` to our disposal, we miss the `dirname()`\n> function.\n> \n> So far, we only had one user of that function: credential-cache--daemon\n> (which was only compiled when Unix sockets are available, anyway). But\n> now we also have `builtin/am.c` as user, so we need it.\n\nYes, many moons ago (on my old 32-bit laptop) when I was still 'working'\nwith MinGW I noticed this same thing while looking into providing a win32\nemulation of unix sockets. So, I had to look into this at the same time.\nSince this didn't progress, I didn't mention the libgen issue.\n\nAnyway, I still have a 'test-libgen.c' file (attached) from back then that\ncontains some tests. I don't quite recall what the final state of this\ncode was, but it was intended to test _existing_ libgen implementations\nas well as provide a 'git' version which would work on MinGW, cygwin and\nlinux. Note that some of the existing implementations didn't all agree on\nwhat the tests should report! I don't remember if I looked at the POSIX\nspec or not.\n\nSo, I don't know how useful it will be - if nothing else, there are some\ntests! :-D\n\nHTH\n\nRamsay Jones\n\n\n> \n> Since `dirname()` is a sibling of `basename()`, we simply put our very\n> own `gitdirname()` implementation next to `gitbasename()` and use it\n> if `NO_LIBGEN_H` has been set.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> \n> \tI stumbled over the compile warning when upgrading Git for Windows\n> \tto 2.6.0. There was a left-over NO_LIBGEN_H=YesPlease (which we\n> \tno longer need in Git for Windows 2.x), but it did point to the\n> \tfact that we use `dirname()` in builtin/am.c now, so we better\n> \thave a fall-back implementation for platforms without libgen.h.\n> \n> \tI tested this implementation a bit, but I still would appreciate\n> \ta few eye-balls to go over it.\n> \n> compat/basename.c | 26 ++++++++++++++++++++++++++\n> git-compat-util.h | 2 ++\n> 2 files changed, 28 insertions(+)\n> \n> diff --git a/compat/basename.c b/compat/basename.c\n> index d8f8a3c..10dba38 100644\n> --- a/compat/basename.c\n> +++ b/compat/basename.c\n> @@ -13,3 +13,29 @@ char *gitbasename (char *path)\n> \t}\n> \treturn (char *)base;\n> }\n> +\n> +char *gitdirname(char *path)\n> +{\n> +\tchar *p = path, *slash, c;\n> +\n> +\t/* Skip over the disk name in MSDOS pathnames. */\n> +\tif (has_dos_drive_prefix(p))\n> +\t\tp += 2;\n> +\t/* POSIX.1-2001 says dirname(\"/\") should return \"/\" */\n> +\tslash = is_dir_sep(*p) ? ++p : NULL;\n> +\twhile ((c = *(p++)))\n> +\t\tif (is_dir_sep(c)) {\n> +\t\t\tchar *tentative = p - 1;\n> +\n> +\t\t\t/* POSIX.1-2001 says to ignore trailing slashes */\n> +\t\t\twhile (is_dir_sep(*p))\n> +\t\t\t\tp++;\n> +\t\t\tif (*p)\n> +\t\t\t\tslash = tentative;\n> +\t\t}\n> +\n> +\tif (!slash)\n> +\t\treturn \".\";\n> +\t*slash = '\\0';\n> +\treturn path;\n> +}\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index f649e81..8b01aa5 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -253,6 +253,8 @@ struct itimerval {\n> #else\n> #define basename gitbasename\n> extern char *gitbasename(char *);\n> +#define dirname gitdirname\n> +extern char *gitdirname(char *);\n> #endif\n> \n> #ifndef NO_ICONV\n> \n\n\n#include \n#include \n#include \n#ifndef NO_LIBGEN_H\n# include \n#endif\n\nstruct test_data {\n\tchar *from; /* input: transform from this ... */\n\tchar *to; /* output: ... to this. */\n};\n\n#ifdef NO_LIBGEN_H\n\n#if defined(__MINGW32__) || defined(_MSC_VER)\n#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')\n#define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n#else\n#define has_dos_drive_prefix(path) 0\n#define is_dir_sep(c) ((c) == '/')\n#endif\n\n#define basename gitbasename\n#define dirname gitdirname\n\nchar *gitbasename (char *path)\n{\n\tchar *p;\n\n\tif (!path || !*path)\n\t\treturn \".\";\n\t/* skip drive designator, if any */\n\tif (has_dos_drive_prefix(path))\n\t\tpath += 2;\n\tif (!*path)\n\t\treturn \".\";\n\t/* trim trailing directory separators */\n\tp = path + strlen(path) - 1;\n\twhile (is_dir_sep(*p)) {\n\t\tif (p == path)\n\t\t\treturn path;\n\t\t*p-- = '\\0';\n\t}\n\t/* find begining of last path component */\n\twhile (p >= path && !is_dir_sep(*p))\n\t\tp--;\n\treturn p + 1;\n}\n\nchar *gitdirname(char *path)\n{\n\tchar *p, *start;\n\n\tif (!path || !*path)\n\t\treturn \".\";\n\tstart = path;\n\t/* skip drive designator, if any */\n\tif (has_dos_drive_prefix(path))\n\t\tstart += 2;\n\t/* check for // */\n\tif (strcmp(start, \"//\") == 0)\n\t\treturn path;\n\t/* check for \\\\ */\n\tif (is_dir_sep('\\\\') && strcmp(start, \"\\\\\\\\\") == 0)\n\t\treturn path;\n\t/* trim trailing directory separators */\n\tp = path + strlen(path) - 1;\n\twhile (is_dir_sep(*p)) {\n\t\tif (p == start)\n\t\t\treturn path;\n\t\t*p-- = '\\0';\n\t}\n\t/* find begining of last path component */\n\twhile (p >= start && !is_dir_sep(*p))\n\t\tp--;\n\t/* terminate dirname */\n\tif (p < start) {\n\t\tp = start;\n\t\t*p++ = '.';\n\t} else if (p == start)\n\t\tp++;\n\t*p = '\\0';\n\treturn path;\n}\n\n#endif\n\nstatic int test_basename(void)\n{\n\tstatic struct test_data t[] = {\n\n\t\t/* --- POSIX type paths --- */\n\t\t{ NULL, \".\" },\n\t\t{ \"\", \".\" },\n\t\t{ \".\", \".\" },\n\t\t{ \"..\", \"..\" },\n\t\t{ \"/\", \"/\" },\n#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n\t\t{ \"//\", \"//\" },\n\t\t{ \"///\", \"//\" },\n\t\t{ \"////\", \"//\" },\n#else\n\t\t{ \"//\", \"/\" },\n\t\t{ \"///\", \"/\" },\n\t\t{ \"////\", \"/\" },\n#endif\n\t\t{ \"usr\", \"usr\" },\n\t\t{ \"/usr\", \"usr\" },\n\t\t{ \"/usr/\", \"usr\" },\n\t\t{ \"/usr//\", \"usr\" },\n\t\t{ \"/usr/lib\", \"lib\" },\n\t\t{ \"usr/lib\", \"lib\" },\n\t\t{ \"usr/lib///\", \"lib\" },\n\n#if defined(__MINGW32__) || defined(_MSC_VER)\n\n\t\t/* --- win32 type paths --- */\n\t\t{ \"\\\\usr\", \"usr\" },\n\t\t{ \"\\\\usr\\\\\", \"usr\" },\n\t\t{ \"\\\\usr\\\\\\\\\", \"usr\" },\n\t\t{ \"\\\\usr\\\\lib\", \"lib\" },\n\t\t{ \"usr\\\\lib\", \"lib\" },\n\t\t{ \"usr\\\\lib\\\\\\\\\\\\\", \"lib\" },\n\t\t{ \"C:/usr\", \"usr\" },\n\t\t{ \"C:/usr\", \"usr\" },\n\t\t{ \"C:/usr/\", \"usr\" },\n\t\t{ \"C:/usr//\", \"usr\" },\n\t\t{ \"C:/usr/lib\", \"lib\" },\n\t\t{ \"C:usr/lib\", \"lib\" },\n\t\t{ \"C:usr/lib///\", \"lib\" },\n\t\t{ \"C:\", \".\" },\n\t\t{ \"C:a\", \"a\" },\n\t\t{ \"C:/\", \"/\" },\n\t\t{ \"C:///\", \"/\" },\n#if defined(NO_LIBGEN_H)\n\t\t{ \"\\\\\", \"\\\\\" },\n\t\t{ \"\\\\\\\\\", \"\\\\\" },\n\t\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n#else\n\n\t\t/* win32 platform variations: */\n#if defined(__MINGW32__)\n\t\t{ \"\\\\\", \"/\" },\n\t\t{ \"\\\\\\\\\", \"/\" },\n\t\t{ \"\\\\\\\\\\\\\", \"/\" },\n#endif\n\n#if defined(_MSC_VER)\n\t\t{ \"\\\\\", \"\\\\\" },\n\t\t{ \"\\\\\\\\\", \"\\\\\" },\n\t\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n#endif\n\n#endif\n#endif\n\t\t{ NULL, \".\" }\n\t};\n\tstatic char input[1024];\n\tchar *from, *to;\n\tint i, failed = 0;\n\n\tfor (i = 0; i < sizeof(t)/sizeof(t[0]); i++) {\n\t\tfrom = NULL;\n\t\tif (t[i].from) {\n\t\t\tstrcpy(input, t[i].from);\n\t\t\tfrom = input;\n\t\t}\n\t\tto = basename(from);\n\t\tif (strcmp(to, t[i].to) != 0) {\n\t\t\tfprintf(stderr, \"FAIL: basename(%s) => '%s' != '%s'\\n\",\n\t\t\t\tt[i].from, to, t[i].to);\n\t\t\tfailed++;\n\t\t}\n\t}\n\treturn failed != 0;\n}\n\nstatic int test_dirname(void)\n{\n\tstatic struct test_data t[] = {\n\n\t\t/* --- POSIX type paths --- */\n\t\t{ NULL, \".\" },\n\t\t{ \"\", \".\" },\n\t\t{ \".\", \".\" },\n\t\t{ \"..\", \".\" },\n\t\t{ \"/\", \"/\" },\n\t\t{ \"//\", \"//\" },\n#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n\t\t{ \"///\", \"//\" },\n\t\t{ \"////\", \"//\" },\n#else\n\t\t{ \"///\", \"/\" },\n\t\t{ \"////\", \"/\" },\n#endif\n\t\t{ \"usr\", \".\" },\n\t\t{ \"/usr\", \"/\" },\n\t\t{ \"/usr/\", \"/\" },\n\t\t{ \"/usr//\", \"/\" },\n\t\t{ \"/usr/lib\", \"/usr\" },\n\t\t{ \"usr/lib\", \"usr\" },\n\t\t{ \"usr/lib///\", \"usr\" },\n\n#if defined(__MINGW32__) || defined(_MSC_VER)\n\n\t\t/* --- win32 type paths --- */\n\t\t{ \"\\\\\", \"\\\\\" },\n\t\t{ \"\\\\\\\\\", \"\\\\\\\\\" },\n\t\t{ \"\\\\usr\", \"\\\\\" },\n\t\t{ \"\\\\usr\\\\\", \"\\\\\" },\n\t\t{ \"\\\\usr\\\\\\\\\", \"\\\\\" },\n\t\t{ \"\\\\usr\\\\lib\", \"\\\\usr\" },\n\t\t{ \"usr\\\\lib\", \"usr\" },\n\t\t{ \"usr\\\\lib\\\\\\\\\\\\\", \"usr\" },\n\t\t{ \"C:a\", \"C:.\" },\n\t\t{ \"C:/\", \"C:/\" },\n\t\t{ \"C:///\", \"C:/\" },\n\t\t{ \"C:/usr\", \"C:/\" },\n\t\t{ \"C:/usr/\", \"C:/\" },\n\t\t{ \"C:/usr//\", \"C:/\" },\n\t\t{ \"C:/usr/lib\", \"C:/usr\" },\n\t\t{ \"C:usr/lib\", \"C:usr\" },\n\t\t{ \"C:usr/lib///\", \"C:usr\" },\n\t\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n\t\t{ \"\\\\\\\\\\\\\\\\\", \"\\\\\" },\n#if defined(NO_LIBGEN_H)\n\t\t{ \"C:\", \"C:.\" },\n#else\n\n\t\t/* win32 platform variations: */\n#if defined(__MINGW32__)\n\t\t/* the following is clearly wrong ... */\n\t\t{ \"C:\", \".\" },\n#endif\n\n#if defined(_MSC_VER)\n\t\t{ \"C:\", \"C:.\" },\n#endif\n\n#endif\n#endif\n\t\t{ NULL, \".\" }\n\t};\n\tstatic char input[1024];\n\tchar *from, *to;\n\tint i, failed = 0;\n\n\tfor (i = 0; i < sizeof(t)/sizeof(t[0]); i++) {\n\t\tfrom = NULL;\n\t\tif (t[i].from) {\n\t\t\tstrcpy(input, t[i].from);\n\t\t\tfrom = input;\n\t\t}\n\t\tto = dirname(from);\n\t\tif (strcmp(to, t[i].to) != 0) {\n\t\t\tfprintf(stderr, \"FAIL: dirname(%s) => '%s' != '%s'\\n\",\n\t\t\t\tt[i].from, to, t[i].to);\n\t\t\tfailed++;\n\t\t}\n\t}\n\treturn failed != 0;\n}\n\nint main(int argc, char **argv)\n{\n\tif (argc == 2 && !strcmp(argv[1], \"basename\"))\n\t\treturn test_basename();\n\n\tif (argc == 2 && !strcmp(argv[1], \"dirname\"))\n\t\treturn test_dirname();\n\n\tfprintf(stderr, \"%s: unknown function name: %s\\n\", argv[0],\n\t\targv[1] ? argv[1] : \"(there was none)\");\n\treturn 1;\n}\n"},{"id":"275556","messageId":"alpine.DEB.2.20.1601081707340.2964@virtualbox","threadId":"40458","inReplyTo":"xmqq8u7n934i.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-08T16:17:29Z","receivedAt":"2016-01-08T16:17:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 30 Sep 2015, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > diff --git a/compat/basename.c b/compat/basename.c\n> > index d8f8a3c..10dba38 100644\n> > --- a/compat/basename.c\n> > +++ b/compat/basename.c\n> > @@ -13,3 +13,29 @@ char *gitbasename (char *path)\n> > \t}\n> > \treturn (char *)base;\n> > }\n> > +\n> > +char *gitdirname(char *path)\n> > +{\n> > +\tchar *p = path, *slash, c;\n> > +\n> > +\t/* Skip over the disk name in MSDOS pathnames. */\n> > +\tif (has_dos_drive_prefix(p))\n> > +\t\tp += 2;\n> \n> Not a new problem, but many callers of has_dos_drive_prefix()\n> hardcodes that \"2\" in various forms. I wonder if this is something\n> we should relieve callers of by tweaking the semantics of it, e.g.\n> by returning 2 (or howmanyever bytes should be skipped) from the\n> function, changing it to skip_dos_drive_prefix(&p), etc.\n\nIn the upcoming v2, this is addressed.\n\n> > +\t/* POSIX.1-2001 says dirname(\"/\") should return \"/\" */\n> > +\tslash = is_dir_sep(*p) ? ++p : NULL;\n> > +\twhile ((c = *(p++)))\n> \n> I am confused by this. What is the invariant on 'p' at the\n> beginning of the body of this while loop in each iteration?\n\nThe idea was that 'p' looks at whatever is the next character. And 'slash'\nrecords the location of the latest slash we have seen (to be overwritten\nwith a '\\0' to terminate the dirname).\n\nSince '/' should not be shortened to the empty string, I special-cased\nabsolute paths to actually shift the 'slash' to one byte later. A little\ndirty, but it worked.\n\nExcept that Ramsay's tests pointed out that I did not even look at what\nthe specs say, and I even fixed basename() in the meantime (and I think\ndirname() looks more readable, but feel free to contradict me there).\n\n> > +\t\tif (is_dir_sep(c)) {\n> > +\t\t\tchar *tentative = p - 1;\n> > +\n> > +\t\t\t/* POSIX.1-2001 says to ignore trailing slashes */\n> > +\t\t\twhile (is_dir_sep(*p))\n> > +\t\t\t\tp++;\n> > +\t\t\tif (*p)\n> > +\t\t\t\tslash = tentative;\n> > +\t\t}\n> \n> I would have expected the function to scan from the end/right/tail.\n\nWhy scan twice (once to find the end, then to find the slashes)?\n\nCiao,\nDscho\n"},{"id":"275557","messageId":"alpine.DEB.2.20.1601081717430.2964@virtualbox","threadId":"40458","inReplyTo":"560C30B1.3010508@ramsayjones.plus.com","subject":"Re: [PATCH] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-08T16:18:51Z","receivedAt":"2016-01-08T16:18:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ramsay,\n\nOn Wed, 30 Sep 2015, Ramsay Jones wrote:\n\n> On 30/09/15 15:50, Johannes Schindelin wrote:\n> > When there is no `libgen.h` to our disposal, we miss the `dirname()`\n> > function.\n> > \n> > So far, we only had one user of that function:\n> > credential-cache--daemon (which was only compiled when Unix sockets\n> > are available, anyway). But now we also have `builtin/am.c` as user,\n> > so we need it.\n> \n> Yes, many moons ago (on my old 32-bit laptop) when I was still 'working'\n> with MinGW I noticed this same thing while looking into providing a win32\n> emulation of unix sockets. So, I had to look into this at the same time.\n> Since this didn't progress, I didn't mention the libgen issue.\n> \n> Anyway, I still have a 'test-libgen.c' file (attached) from back then that\n> contains some tests.\n\nAwesome. Thank you! I integrated the tests back into test-path-utils.c\n(from where the framework clearly came) and made it part of the regression\ntest suite in the upcoming v2.\n\nCiao,\nDscho\n"},{"id":"275562","messageId":"cover.1452270051.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"25a2598e756959f55f06ae6b4dc6f448e3b6b127.1443624188.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 0/4] Ensure that we can build without libgen.h","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-08T16:21:07Z","receivedAt":"2016-01-08T16:21:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This mini series adds a fall-back for the `dirname()` function that we use\ne.g. in git-am. This is necessary because not all platforms have a working\nlibgen.h. MSys2 (on which Git for Windows relies) does have a libgen.h, but\nits `basename()` implementation is broken, thus we cannot use it.\n\n\nJohannes Schindelin (4):\n Refactor skipping DOS drive prefixes\n compat/basename: make basename() conform to POSIX\n Provide a dirname() function when NO_LIBGEN_H=YesPlease\n t0060: verify that basename() and dirname() work as expected\n\n compat/basename.c | 66 ++++++++++++++++++--\n compat/mingw.c | 14 ++---\n compat/mingw.h | 10 ++-\n git-compat-util.h | 10 +++\n path.c | 14 ++---\n t/t0060-path-utils.sh | 3 +\n test-path-utils.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 7 files changed, 261 insertions(+), 24 deletions(-)\n\nInterdiff vs v1:\n\n diff --git a/compat/basename.c b/compat/basename.c\n index 10dba38..0a2ed25 100644\n --- a/compat/basename.c\n +++ b/compat/basename.c\n @@ -1,28 +1,58 @@\n #include \"../git-compat-util.h\"\n +#include \"../strbuf.h\"\n \n /* Adapted from libiberty's basename.c. */\n char *gitbasename (char *path)\n {\n \tconst char *base;\n -\t/* Skip over the disk name in MSDOS pathnames. */\n -\tif (has_dos_drive_prefix(path))\n -\t\tpath += 2;\n +\n +\tif (path)\n +\t\tskip_dos_drive_prefix(&path);\n +\n +\tif (!path || !*path)\n +\t\treturn \".\";\n +\n \tfor (base = path; *path; path++) {\n -\t\tif (is_dir_sep(*path))\n -\t\t\tbase = path + 1;\n +\t\tif (!is_dir_sep(*path))\n +\t\t\tcontinue;\n +\t\tdo {\n +\t\t\tpath++;\n +\t\t} while (is_dir_sep(*path));\n +\t\tif (*path)\n +\t\t\tbase = path;\n +\t\telse\n +\t\t\twhile (--path != base && is_dir_sep(*path))\n +\t\t\t\t*path = '\\0';\n \t}\n \treturn (char *)base;\n }\n \n char *gitdirname(char *path)\n {\n -\tchar *p = path, *slash, c;\n +\tchar *p = path, *slash = NULL, c;\n +\tint dos_drive_prefix;\n +\n +\tif (!p)\n +\t\treturn \".\";\n \n -\t/* Skip over the disk name in MSDOS pathnames. */\n -\tif (has_dos_drive_prefix(p))\n -\t\tp += 2;\n -\t/* POSIX.1-2001 says dirname(\"/\") should return \"/\" */\n -\tslash = is_dir_sep(*p) ? ++p : NULL;\n +\tif ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p) {\n +\t\tstatic struct strbuf buf = STRBUF_INIT;\n +\n +dot:\n +\t\tstrbuf_reset(&buf);\n +\t\tstrbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n +\t\treturn buf.buf;\n +\t}\n +\n +\t/*\n +\t * POSIX.1-2001 says dirname(\"/\") should return \"/\", and dirname(\"//\")\n +\t * should return \"//\", but dirname(\"///\") should return \"/\" again.\n +\t */\n +\tif (is_dir_sep(*p)) {\n +\t\tif (!p[1] || (is_dir_sep(p[1]) && !p[2]))\n +\t\t\treturn path;\n +\t\tslash = ++p;\n +\t}\n \twhile ((c = *(p++)))\n \t\tif (is_dir_sep(c)) {\n \t\t\tchar *tentative = p - 1;\n @@ -35,7 +65,7 @@ char *gitdirname(char *path)\n \t\t}\n \n \tif (!slash)\n -\t\treturn \".\";\n +\t\tgoto dot;\n \t*slash = '\\0';\n \treturn path;\n }\n diff --git a/compat/mingw.c b/compat/mingw.c\n index 5edea29..1b3530a 100644\n --- a/compat/mingw.c\n +++ b/compat/mingw.c\n @@ -1934,26 +1934,22 @@ pid_t waitpid(pid_t pid, int *status, int options)\n \n int mingw_offset_1st_component(const char *path)\n {\n -\tint offset = 0;\n -\tif (has_dos_drive_prefix(path))\n -\t\toffset = 2;\n +\tchar *pos = (char *)path;\n \n \t/* unc paths */\n -\telse if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {\n -\n +\tif (!skip_dos_drive_prefix(&pos) &&\n +\t\t\tis_dir_sep(pos[0]) && is_dir_sep(pos[1])) {\n \t\t/* skip server name */\n -\t\tchar *pos = strpbrk(path + 2, \"\\\\/\");\n +\t\tpos = strpbrk(pos + 2, \"\\\\/\");\n \t\tif (!pos)\n \t\t\treturn 0; /* Error: malformed unc path */\n \n \t\tdo {\n \t\t\tpos++;\n \t\t} while (*pos && !is_dir_sep(*pos));\n -\n -\t\toffset = pos - path;\n \t}\n \n -\treturn offset + is_dir_sep(path[offset]);\n +\treturn pos + is_dir_sep(*pos) - path;\n }\n \n int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)\n diff --git a/compat/mingw.h b/compat/mingw.h\n index 57ca477..b3e5044 100644\n --- a/compat/mingw.h\n +++ b/compat/mingw.h\n @@ -361,7 +361,15 @@ HANDLE winansi_get_osfhandle(int fd);\n * git specific compatibility\n */\n \n -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')\n +#define has_dos_drive_prefix(path) \\\n +\t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n +static inline int mingw_skip_dos_drive_prefix(char **path)\n +{\n +\tint ret = has_dos_drive_prefix(*path);\n +\t*path += ret;\n +\treturn ret;\n +}\n +#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n static inline char *mingw_find_last_dir_sep(const char *path)\n {\n diff --git a/git-compat-util.h b/git-compat-util.h\n index 996ee17..94f311a 100644\n --- a/git-compat-util.h\n +++ b/git-compat-util.h\n @@ -337,6 +337,14 @@ static inline int git_has_dos_drive_prefix(const char *path)\n #define has_dos_drive_prefix git_has_dos_drive_prefix\n #endif\n \n +#ifndef skip_dos_drive_prefix\n +static inline int git_skip_dos_drive_prefix(const char **path)\n +{\n +\treturn 0;\n +}\n +#define skip_dos_drive_prefix git_skip_dos_drive_prefix\n +#endif\n +\n #ifndef is_dir_sep\n static inline int git_is_dir_sep(int c)\n {\n diff --git a/path.c b/path.c\n index 3cd155e..8b7e168 100644\n --- a/path.c\n +++ b/path.c\n @@ -782,13 +782,10 @@ const char *relative_path(const char *in, const char *prefix,\n \telse if (!prefix_len)\n \t\treturn in;\n \n -\tif (have_same_root(in, prefix)) {\n +\tif (have_same_root(in, prefix))\n \t\t/* bypass dos_drive, for \"c:\" is identical to \"C:\" */\n -\t\tif (has_dos_drive_prefix(in)) {\n -\t\t\ti = 2;\n -\t\t\tj = 2;\n -\t\t}\n -\t} else {\n +\t\ti = j = has_dos_drive_prefix(in);\n +\telse {\n \t\treturn in;\n \t}\n \n @@ -943,11 +940,10 @@ const char *remove_leading_path(const char *in, const char *prefix)\n int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n {\n \tchar *dst0;\n +\tint i;\n \n -\tif (has_dos_drive_prefix(src)) {\n +\tfor (i = has_dos_drive_prefix(src); i > 0; i--)\n \t\t*dst++ = *src++;\n -\t\t*dst++ = *src++;\n -\t}\n \tdst0 = dst;\n \n \tif (is_dir_sep(*src)) {\n diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\n index 627ef85..f0152a7 100755\n --- a/t/t0060-path-utils.sh\n +++ b/t/t0060-path-utils.sh\n @@ -59,6 +59,9 @@ case $(uname -s) in\n \t;;\n esac\n \n +test_expect_success basename 'test-path-utils basename'\n +test_expect_success dirname 'test-path-utils dirname'\n +\n norm_path \"\" \"\"\n norm_path . \"\"\n norm_path ./ \"\"\n diff --git a/test-path-utils.c b/test-path-utils.c\n index c67bf65..74e74c9 100644\n --- a/test-path-utils.c\n +++ b/test-path-utils.c\n @@ -39,6 +39,168 @@ static void normalize_argv_string(const char **var, const char *input)\n \t\tdie(\"Bad value: %s\\n\", input);\n }\n \n +struct test_data {\n +\tchar *from; /* input: transform from this ... */\n +\tchar *to; /* output: ... to this. */\n +};\n +\n +static int test_function(struct test_data *data, char *(*func)(char *input),\n +\tconst char *funcname)\n +{\n +\tint failed = 0, i;\n +\tstatic char buffer[1024];\n +\tchar *to;\n +\n +\tfor (i = 0; data[i].to; i++) {\n +\t\tif (!data[i].from)\n +\t\t\tto = func(NULL);\n +\t\telse {\n +\t\t\tstrcpy(buffer, data[i].from);\n +\t\t\tto = func(buffer);\n +\t\t}\n +\t\tif (strcmp(to, data[i].to)) {\n +\t\t\terror(\"FAIL: %s(%s) => '%s' != '%s'\\n\",\n +\t\t\t\tfuncname, data[i].from, to, data[i].to);\n +\t\t\tfailed++;\n +\t\t}\n +\t}\n +\treturn !!failed;\n +}\n +\n +static struct test_data basename_data[] = {\n +\t/* --- POSIX type paths --- */\n +\t{ NULL, \".\" },\n +\t{ \"\", \".\" },\n +\t{ \".\", \".\" },\n +\t{ \"..\", \"..\" },\n +\t{ \"/\", \"/\" },\n +#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n +\t{ \"//\", \"//\" },\n +\t{ \"///\", \"//\" },\n +\t{ \"////\", \"//\" },\n +#else\n +\t{ \"//\", \"/\" },\n +\t{ \"///\", \"/\" },\n +\t{ \"////\", \"/\" },\n +#endif\n +\t{ \"usr\", \"usr\" },\n +\t{ \"/usr\", \"usr\" },\n +\t{ \"/usr/\", \"usr\" },\n +\t{ \"/usr//\", \"usr\" },\n +\t{ \"/usr/lib\", \"lib\" },\n +\t{ \"usr/lib\", \"lib\" },\n +\t{ \"usr/lib///\", \"lib\" },\n +\n +#if defined(__MINGW32__) || defined(_MSC_VER)\n +\n +\t/* --- win32 type paths --- */\n +\t{ \"\\\\usr\", \"usr\" },\n +\t{ \"\\\\usr\\\\\", \"usr\" },\n +\t{ \"\\\\usr\\\\\\\\\", \"usr\" },\n +\t{ \"\\\\usr\\\\lib\", \"lib\" },\n +\t{ \"usr\\\\lib\", \"lib\" },\n +\t{ \"usr\\\\lib\\\\\\\\\\\\\", \"lib\" },\n +\t{ \"C:/usr\", \"usr\" },\n +\t{ \"C:/usr\", \"usr\" },\n +\t{ \"C:/usr/\", \"usr\" },\n +\t{ \"C:/usr//\", \"usr\" },\n +\t{ \"C:/usr/lib\", \"lib\" },\n +\t{ \"C:usr/lib\", \"lib\" },\n +\t{ \"C:usr/lib///\", \"lib\" },\n +\t{ \"C:\", \".\" },\n +\t{ \"C:a\", \"a\" },\n +\t{ \"C:/\", \"/\" },\n +\t{ \"C:///\", \"/\" },\n +#if defined(NO_LIBGEN_H)\n +\t{ \"\\\\\", \"\\\\\" },\n +\t{ \"\\\\\\\\\", \"\\\\\" },\n +\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n +#else\n +\n +\t/* win32 platform variations: */\n +#if defined(__MINGW32__)\n +\t{ \"\\\\\", \"/\" },\n +\t{ \"\\\\\\\\\", \"/\" },\n +\t{ \"\\\\\\\\\\\\\", \"/\" },\n +#endif\n +\n +#if defined(_MSC_VER)\n +\t{ \"\\\\\", \"\\\\\" },\n +\t{ \"\\\\\\\\\", \"\\\\\" },\n +\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n +#endif\n +\n +#endif\n +#endif\n +\t{ NULL, \".\" },\n +\t{ NULL, NULL }\n +};\n +\n +static struct test_data dirname_data[] = {\n +\t/* --- POSIX type paths --- */\n +\t{ NULL, \".\" },\n +\t{ \"\", \".\" },\n +\t{ \".\", \".\" },\n +\t{ \"..\", \".\" },\n +\t{ \"/\", \"/\" },\n +\t{ \"//\", \"//\" },\n +#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n +\t{ \"///\", \"//\" },\n +\t{ \"////\", \"//\" },\n +#else\n +\t{ \"///\", \"/\" },\n +\t{ \"////\", \"/\" },\n +#endif\n +\t{ \"usr\", \".\" },\n +\t{ \"/usr\", \"/\" },\n +\t{ \"/usr/\", \"/\" },\n +\t{ \"/usr//\", \"/\" },\n +\t{ \"/usr/lib\", \"/usr\" },\n +\t{ \"usr/lib\", \"usr\" },\n +\t{ \"usr/lib///\", \"usr\" },\n +\n +#if defined(__MINGW32__) || defined(_MSC_VER)\n +\n +\t/* --- win32 type paths --- */\n +\t{ \"\\\\\", \"\\\\\" },\n +\t{ \"\\\\\\\\\", \"\\\\\\\\\" },\n +\t{ \"\\\\usr\", \"\\\\\" },\n +\t{ \"\\\\usr\\\\\", \"\\\\\" },\n +\t{ \"\\\\usr\\\\\\\\\", \"\\\\\" },\n +\t{ \"\\\\usr\\\\lib\", \"\\\\usr\" },\n +\t{ \"usr\\\\lib\", \"usr\" },\n +\t{ \"usr\\\\lib\\\\\\\\\\\\\", \"usr\" },\n +\t{ \"C:a\", \"C:.\" },\n +\t{ \"C:/\", \"C:/\" },\n +\t{ \"C:///\", \"C:/\" },\n +\t{ \"C:/usr\", \"C:/\" },\n +\t{ \"C:/usr/\", \"C:/\" },\n +\t{ \"C:/usr//\", \"C:/\" },\n +\t{ \"C:/usr/lib\", \"C:/usr\" },\n +\t{ \"C:usr/lib\", \"C:usr\" },\n +\t{ \"C:usr/lib///\", \"C:usr\" },\n +\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n +\t{ \"\\\\\\\\\\\\\\\\\", \"\\\\\" },\n +#if defined(NO_LIBGEN_H)\n +\t{ \"C:\", \"C:.\" },\n +#else\n +\n +\t/* win32 platform variations: */\n +#if defined(__MINGW32__)\n +\t/* the following is clearly wrong ... */\n +\t{ \"C:\", \".\" },\n +#endif\n +\n +#if defined(_MSC_VER)\n +\t{ \"C:\", \"C:.\" },\n +#endif\n +\n +#endif\n +#endif\n +\t{ NULL, \".\" },\n +\t{ NULL, NULL }\n +};\n +\n int main(int argc, char **argv)\n {\n \tif (argc == 3 && !strcmp(argv[1], \"normalize_path_copy\")) {\n @@ -133,6 +295,12 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n \n +\tif (argc == 2 && !strcmp(argv[1], \"basename\"))\n +\t\treturn test_function(basename_data, basename, argv[1]);\n +\n +\tif (argc == 2 && !strcmp(argv[1], \"dirname\"))\n +\t\treturn test_function(dirname_data, dirname, argv[1]);\n +\n \tfprintf(stderr, \"%s: unknown function name: %s\\n\", argv[0],\n \t\targv[1] ? argv[1] : \"(there was none)\");\n \treturn 1;\n\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275558","messageId":"c70ed05f275a44fbfae831b4cb67e59a0ce05724.1452270051.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452270051.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 1/4] Refactor skipping DOS drive prefixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-08T16:21:11Z","receivedAt":"2016-01-08T16:21:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Junio Hamano pointed out that there is an implicit assumption in pretty\nmuch all the code calling has_dos_drive_prefix(): it assumes that the\nDOS drive prefix is always two bytes long.\n\nWhile this assumption is pretty safe, we can still make the code more\nreadable and less error-prone by introducing a function that skips the\nDOS drive prefix safely.\n\nWhile at it, we change the has_dos_drive_prefix() return value: it now\nreturns the number of bytes to be skipped if there is a DOS drive prefix.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/basename.c | 4 +---\n compat/mingw.c | 14 +++++---------\n compat/mingw.h | 10 +++++++++-\n git-compat-util.h | 8 ++++++++\n path.c | 14 +++++---------\n 5 files changed, 28 insertions(+), 22 deletions(-)\n\ndiff --git a/compat/basename.c b/compat/basename.c\nindex d8f8a3c..9f00421 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -4,9 +4,7 @@\n char *gitbasename (char *path)\n {\n \tconst char *base;\n-\t/* Skip over the disk name in MSDOS pathnames. */\n-\tif (has_dos_drive_prefix(path))\n-\t\tpath += 2;\n+\tskip_dos_drive_prefix(&path);\n \tfor (base = path; *path; path++) {\n \t\tif (is_dir_sep(*path))\n \t\t\tbase = path + 1;\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 5edea29..1b3530a 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1934,26 +1934,22 @@ pid_t waitpid(pid_t pid, int *status, int options)\n \n int mingw_offset_1st_component(const char *path)\n {\n-\tint offset = 0;\n-\tif (has_dos_drive_prefix(path))\n-\t\toffset = 2;\n+\tchar *pos = (char *)path;\n \n \t/* unc paths */\n-\telse if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {\n-\n+\tif (!skip_dos_drive_prefix(&pos) &&\n+\t\t\tis_dir_sep(pos[0]) && is_dir_sep(pos[1])) {\n \t\t/* skip server name */\n-\t\tchar *pos = strpbrk(path + 2, \"\\\\/\");\n+\t\tpos = strpbrk(pos + 2, \"\\\\/\");\n \t\tif (!pos)\n \t\t\treturn 0; /* Error: malformed unc path */\n \n \t\tdo {\n \t\t\tpos++;\n \t\t} while (*pos && !is_dir_sep(*pos));\n-\n-\t\toffset = pos - path;\n \t}\n \n-\treturn offset + is_dir_sep(path[offset]);\n+\treturn pos + is_dir_sep(*pos) - path;\n }\n \n int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 57ca477..b3e5044 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -361,7 +361,15 @@ HANDLE winansi_get_osfhandle(int fd);\n * git specific compatibility\n */\n \n-#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')\n+#define has_dos_drive_prefix(path) \\\n+\t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n+static inline int mingw_skip_dos_drive_prefix(char **path)\n+{\n+\tint ret = has_dos_drive_prefix(*path);\n+\t*path += ret;\n+\treturn ret;\n+}\n+#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n static inline char *mingw_find_last_dir_sep(const char *path)\n {\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 2da0a75..0d66f3a 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -335,6 +335,14 @@ static inline int git_has_dos_drive_prefix(const char *path)\n #define has_dos_drive_prefix git_has_dos_drive_prefix\n #endif\n \n+#ifndef skip_dos_drive_prefix\n+static inline int git_skip_dos_drive_prefix(const char **path)\n+{\n+\treturn 0;\n+}\n+#define skip_dos_drive_prefix git_skip_dos_drive_prefix\n+#endif\n+\n #ifndef is_dir_sep\n static inline int git_is_dir_sep(int c)\n {\ndiff --git a/path.c b/path.c\nindex 3cd155e..8b7e168 100644\n--- a/path.c\n+++ b/path.c\n@@ -782,13 +782,10 @@ const char *relative_path(const char *in, const char *prefix,\n \telse if (!prefix_len)\n \t\treturn in;\n \n-\tif (have_same_root(in, prefix)) {\n+\tif (have_same_root(in, prefix))\n \t\t/* bypass dos_drive, for \"c:\" is identical to \"C:\" */\n-\t\tif (has_dos_drive_prefix(in)) {\n-\t\t\ti = 2;\n-\t\t\tj = 2;\n-\t\t}\n-\t} else {\n+\t\ti = j = has_dos_drive_prefix(in);\n+\telse {\n \t\treturn in;\n \t}\n \n@@ -943,11 +940,10 @@ const char *remove_leading_path(const char *in, const char *prefix)\n int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n {\n \tchar *dst0;\n+\tint i;\n \n-\tif (has_dos_drive_prefix(src)) {\n+\tfor (i = has_dos_drive_prefix(src); i > 0; i--)\n \t\t*dst++ = *src++;\n-\t\t*dst++ = *src++;\n-\t}\n \tdst0 = dst;\n \n \tif (is_dir_sep(*src)) {\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275561","messageId":"abd20a9fb53d702cb878b8fa767881e7c1ef2148.1452270051.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452270051.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 2/4] compat/basename: make basename() conform to POSIX","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-08T16:21:15Z","receivedAt":"2016-01-08T16:21:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"According to POSIX, basename(\"/path/\") should return \"path\", not\n\"path/\". Likewise, basename(NULL) and basename(\"abc\") should both\nreturn \".\".\n\nSigned-off-by: Johannes Schindelin \n---\n compat/basename.c | 20 +++++++++++++++++---\n 1 file changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/compat/basename.c b/compat/basename.c\nindex 9f00421..0f1b0b0 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -4,10 +4,24 @@\n char *gitbasename (char *path)\n {\n \tconst char *base;\n-\tskip_dos_drive_prefix(&path);\n+\n+\tif (path)\n+\t\tskip_dos_drive_prefix(&path);\n+\n+\tif (!path || !*path)\n+\t\treturn \".\";\n+\n \tfor (base = path; *path; path++) {\n-\t\tif (is_dir_sep(*path))\n-\t\t\tbase = path + 1;\n+\t\tif (!is_dir_sep(*path))\n+\t\t\tcontinue;\n+\t\tdo {\n+\t\t\tpath++;\n+\t\t} while (is_dir_sep(*path));\n+\t\tif (*path)\n+\t\t\tbase = path;\n+\t\telse\n+\t\t\twhile (--path != base && is_dir_sep(*path))\n+\t\t\t\t*path = '\\0';\n \t}\n \treturn (char *)base;\n }\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275559","messageId":"4e1b6c602e0adccfc11152b00aa23d14b1bbd4a8.1452270051.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452270051.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-08T16:21:18Z","receivedAt":"2016-01-08T16:21:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When there is no `libgen.h` to our disposal, we miss the `dirname()`\nfunction.\n\nSo far, we only had one user of that function: credential-cache--daemon\n(which was only compiled when Unix sockets are available, anyway). But\nnow we also have `builtin/am.c` as user, so we need it.\n\nSince `dirname()` is a sibling of `basename()`, we simply put our very\nown `gitdirname()` implementation next to `gitbasename()` and use it\nif `NO_LIBGEN_H` has been set.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/basename.c | 44 ++++++++++++++++++++++++++++++++++++++++++++\n git-compat-util.h | 2 ++\n 2 files changed, 46 insertions(+)\n\ndiff --git a/compat/basename.c b/compat/basename.c\nindex 0f1b0b0..0a2ed25 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -1,4 +1,5 @@\n #include \"../git-compat-util.h\"\n+#include \"../strbuf.h\"\n \n /* Adapted from libiberty's basename.c. */\n char *gitbasename (char *path)\n@@ -25,3 +26,46 @@ char *gitbasename (char *path)\n \t}\n \treturn (char *)base;\n }\n+\n+char *gitdirname(char *path)\n+{\n+\tchar *p = path, *slash = NULL, c;\n+\tint dos_drive_prefix;\n+\n+\tif (!p)\n+\t\treturn \".\";\n+\n+\tif ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p) {\n+\t\tstatic struct strbuf buf = STRBUF_INIT;\n+\n+dot:\n+\t\tstrbuf_reset(&buf);\n+\t\tstrbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n+\t\treturn buf.buf;\n+\t}\n+\n+\t/*\n+\t * POSIX.1-2001 says dirname(\"/\") should return \"/\", and dirname(\"//\")\n+\t * should return \"//\", but dirname(\"///\") should return \"/\" again.\n+\t */\n+\tif (is_dir_sep(*p)) {\n+\t\tif (!p[1] || (is_dir_sep(p[1]) && !p[2]))\n+\t\t\treturn path;\n+\t\tslash = ++p;\n+\t}\n+\twhile ((c = *(p++)))\n+\t\tif (is_dir_sep(c)) {\n+\t\t\tchar *tentative = p - 1;\n+\n+\t\t\t/* POSIX.1-2001 says to ignore trailing slashes */\n+\t\t\twhile (is_dir_sep(*p))\n+\t\t\t\tp++;\n+\t\t\tif (*p)\n+\t\t\t\tslash = tentative;\n+\t\t}\n+\n+\tif (!slash)\n+\t\tgoto dot;\n+\t*slash = '\\0';\n+\treturn path;\n+}\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 0d66f3a..94f311a 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -253,6 +253,8 @@ struct itimerval {\n #else\n #define basename gitbasename\n extern char *gitbasename(char *);\n+#define dirname gitdirname\n+extern char *gitdirname(char *);\n #endif\n \n #ifndef NO_ICONV\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275560","messageId":"eca740dbf6271bd69f2ccb14163175996ef7c837.1452270051.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452270051.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-08T16:21:22Z","receivedAt":"2016-01-08T16:21:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Unfortunately, some libgen implementations yield outcomes different from\nwhat Git expects. For example, mingw-w64-crt provides a basename()\nfunction, that shortens `path0/` to `path`!\n\nSo let's verify that the basename() and dirname() functions we use conform\nto what Git expects.\n\nDerived-from-code-by: Ramsay Jones \nSigned-off-by: Johannes Schindelin \n---\n t/t0060-path-utils.sh | 3 +\n test-path-utils.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 171 insertions(+)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 627ef85..f0152a7 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -59,6 +59,9 @@ case $(uname -s) in\n \t;;\n esac\n \n+test_expect_success basename 'test-path-utils basename'\n+test_expect_success dirname 'test-path-utils dirname'\n+\n norm_path \"\" \"\"\n norm_path . \"\"\n norm_path ./ \"\"\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex c67bf65..74e74c9 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -39,6 +39,168 @@ static void normalize_argv_string(const char **var, const char *input)\n \t\tdie(\"Bad value: %s\\n\", input);\n }\n \n+struct test_data {\n+\tchar *from; /* input: transform from this ... */\n+\tchar *to; /* output: ... to this. */\n+};\n+\n+static int test_function(struct test_data *data, char *(*func)(char *input),\n+\tconst char *funcname)\n+{\n+\tint failed = 0, i;\n+\tstatic char buffer[1024];\n+\tchar *to;\n+\n+\tfor (i = 0; data[i].to; i++) {\n+\t\tif (!data[i].from)\n+\t\t\tto = func(NULL);\n+\t\telse {\n+\t\t\tstrcpy(buffer, data[i].from);\n+\t\t\tto = func(buffer);\n+\t\t}\n+\t\tif (strcmp(to, data[i].to)) {\n+\t\t\terror(\"FAIL: %s(%s) => '%s' != '%s'\\n\",\n+\t\t\t\tfuncname, data[i].from, to, data[i].to);\n+\t\t\tfailed++;\n+\t\t}\n+\t}\n+\treturn !!failed;\n+}\n+\n+static struct test_data basename_data[] = {\n+\t/* --- POSIX type paths --- */\n+\t{ NULL, \".\" },\n+\t{ \"\", \".\" },\n+\t{ \".\", \".\" },\n+\t{ \"..\", \"..\" },\n+\t{ \"/\", \"/\" },\n+#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n+\t{ \"//\", \"//\" },\n+\t{ \"///\", \"//\" },\n+\t{ \"////\", \"//\" },\n+#else\n+\t{ \"//\", \"/\" },\n+\t{ \"///\", \"/\" },\n+\t{ \"////\", \"/\" },\n+#endif\n+\t{ \"usr\", \"usr\" },\n+\t{ \"/usr\", \"usr\" },\n+\t{ \"/usr/\", \"usr\" },\n+\t{ \"/usr//\", \"usr\" },\n+\t{ \"/usr/lib\", \"lib\" },\n+\t{ \"usr/lib\", \"lib\" },\n+\t{ \"usr/lib///\", \"lib\" },\n+\n+#if defined(__MINGW32__) || defined(_MSC_VER)\n+\n+\t/* --- win32 type paths --- */\n+\t{ \"\\\\usr\", \"usr\" },\n+\t{ \"\\\\usr\\\\\", \"usr\" },\n+\t{ \"\\\\usr\\\\\\\\\", \"usr\" },\n+\t{ \"\\\\usr\\\\lib\", \"lib\" },\n+\t{ \"usr\\\\lib\", \"lib\" },\n+\t{ \"usr\\\\lib\\\\\\\\\\\\\", \"lib\" },\n+\t{ \"C:/usr\", \"usr\" },\n+\t{ \"C:/usr\", \"usr\" },\n+\t{ \"C:/usr/\", \"usr\" },\n+\t{ \"C:/usr//\", \"usr\" },\n+\t{ \"C:/usr/lib\", \"lib\" },\n+\t{ \"C:usr/lib\", \"lib\" },\n+\t{ \"C:usr/lib///\", \"lib\" },\n+\t{ \"C:\", \".\" },\n+\t{ \"C:a\", \"a\" },\n+\t{ \"C:/\", \"/\" },\n+\t{ \"C:///\", \"/\" },\n+#if defined(NO_LIBGEN_H)\n+\t{ \"\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n+#else\n+\n+\t/* win32 platform variations: */\n+#if defined(__MINGW32__)\n+\t{ \"\\\\\", \"/\" },\n+\t{ \"\\\\\\\\\", \"/\" },\n+\t{ \"\\\\\\\\\\\\\", \"/\" },\n+#endif\n+\n+#if defined(_MSC_VER)\n+\t{ \"\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n+#endif\n+\n+#endif\n+#endif\n+\t{ NULL, \".\" },\n+\t{ NULL, NULL }\n+};\n+\n+static struct test_data dirname_data[] = {\n+\t/* --- POSIX type paths --- */\n+\t{ NULL, \".\" },\n+\t{ \"\", \".\" },\n+\t{ \".\", \".\" },\n+\t{ \"..\", \".\" },\n+\t{ \"/\", \"/\" },\n+\t{ \"//\", \"//\" },\n+#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n+\t{ \"///\", \"//\" },\n+\t{ \"////\", \"//\" },\n+#else\n+\t{ \"///\", \"/\" },\n+\t{ \"////\", \"/\" },\n+#endif\n+\t{ \"usr\", \".\" },\n+\t{ \"/usr\", \"/\" },\n+\t{ \"/usr/\", \"/\" },\n+\t{ \"/usr//\", \"/\" },\n+\t{ \"/usr/lib\", \"/usr\" },\n+\t{ \"usr/lib\", \"usr\" },\n+\t{ \"usr/lib///\", \"usr\" },\n+\n+#if defined(__MINGW32__) || defined(_MSC_VER)\n+\n+\t/* --- win32 type paths --- */\n+\t{ \"\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\", \"\\\\\\\\\" },\n+\t{ \"\\\\usr\", \"\\\\\" },\n+\t{ \"\\\\usr\\\\\", \"\\\\\" },\n+\t{ \"\\\\usr\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\usr\\\\lib\", \"\\\\usr\" },\n+\t{ \"usr\\\\lib\", \"usr\" },\n+\t{ \"usr\\\\lib\\\\\\\\\\\\\", \"usr\" },\n+\t{ \"C:a\", \"C:.\" },\n+\t{ \"C:/\", \"C:/\" },\n+\t{ \"C:///\", \"C:/\" },\n+\t{ \"C:/usr\", \"C:/\" },\n+\t{ \"C:/usr/\", \"C:/\" },\n+\t{ \"C:/usr//\", \"C:/\" },\n+\t{ \"C:/usr/lib\", \"C:/usr\" },\n+\t{ \"C:usr/lib\", \"C:usr\" },\n+\t{ \"C:usr/lib///\", \"C:usr\" },\n+\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\\\\\\\\\", \"\\\\\" },\n+#if defined(NO_LIBGEN_H)\n+\t{ \"C:\", \"C:.\" },\n+#else\n+\n+\t/* win32 platform variations: */\n+#if defined(__MINGW32__)\n+\t/* the following is clearly wrong ... */\n+\t{ \"C:\", \".\" },\n+#endif\n+\n+#if defined(_MSC_VER)\n+\t{ \"C:\", \"C:.\" },\n+#endif\n+\n+#endif\n+#endif\n+\t{ NULL, \".\" },\n+\t{ NULL, NULL }\n+};\n+\n int main(int argc, char **argv)\n {\n \tif (argc == 3 && !strcmp(argv[1], \"normalize_path_copy\")) {\n@@ -133,6 +295,12 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n \n+\tif (argc == 2 && !strcmp(argv[1], \"basename\"))\n+\t\treturn test_function(basename_data, basename, argv[1]);\n+\n+\tif (argc == 2 && !strcmp(argv[1], \"dirname\"))\n+\t\treturn test_function(dirname_data, dirname, argv[1]);\n+\n \tfprintf(stderr, \"%s: unknown function name: %s\\n\", argv[0],\n \t\targv[1] ? argv[1] : \"(there was none)\");\n \treturn 1;\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275568","messageId":"xmqqy4bz29mp.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"abd20a9fb53d702cb878b8fa767881e7c1ef2148.1452270051.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 2/4] compat/basename: make basename() conform to POSIX","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-08T18:45:18Z","receivedAt":"2016-01-08T18:45:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> According to POSIX, basename(\"/path/\") should return \"path\", not\n> \"path/\". Likewise, basename(NULL) and basename(\"abc\") should both\n> return \".\".\n\nDid you mean basename(\"abc\"), not basename(\"\"), here? \n\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> compat/basename.c | 20 +++++++++++++++++---\n> 1 file changed, 17 insertions(+), 3 deletions(-)\n>\n> diff --git a/compat/basename.c b/compat/basename.c\n> index 9f00421..0f1b0b0 100644\n> --- a/compat/basename.c\n> +++ b/compat/basename.c\n> @@ -4,10 +4,24 @@\n> char *gitbasename (char *path)\n> {\n> \tconst char *base;\n> -\tskip_dos_drive_prefix(&path);\n> +\n> +\tif (path)\n> +\t\tskip_dos_drive_prefix(&path);\n> +\n> +\tif (!path || !*path)\n> +\t\treturn \".\";\n> +\n> \tfor (base = path; *path; path++) {\n> -\t\tif (is_dir_sep(*path))\n> -\t\t\tbase = path + 1;\n> +\t\tif (!is_dir_sep(*path))\n> +\t\t\tcontinue;\n> +\t\tdo {\n> +\t\t\tpath++;\n> +\t\t} while (is_dir_sep(*path));\n> +\t\tif (*path)\n> +\t\t\tbase = path;\n> +\t\telse\n> +\t\t\twhile (--path != base && is_dir_sep(*path))\n> +\t\t\t\t*path = '\\0';\n> \t}\n> \treturn (char *)base;\n> }\n"},{"id":"275569","messageId":"xmqqtwmn298r.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"4e1b6c602e0adccfc11152b00aa23d14b1bbd4a8.1452270051.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-08T18:53:40Z","receivedAt":"2016-01-08T18:53:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> When there is no `libgen.h` to our disposal, we miss the `dirname()`\n> function.\n>\n> So far, we only had one user of that function: credential-cache--daemon\n> (which was only compiled when Unix sockets are available, anyway). But\n> now we also have `builtin/am.c` as user, so we need it.\n>\n> Since `dirname()` is a sibling of `basename()`, we simply put our very\n> own `gitdirname()` implementation next to `gitbasename()` and use it\n> if `NO_LIBGEN_H` has been set.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> compat/basename.c | 44 ++++++++++++++++++++++++++++++++++++++++++++\n> git-compat-util.h | 2 ++\n> 2 files changed, 46 insertions(+)\n>\n> diff --git a/compat/basename.c b/compat/basename.c\n> index 0f1b0b0..0a2ed25 100644\n> --- a/compat/basename.c\n> +++ b/compat/basename.c\n> @@ -1,4 +1,5 @@\n> #include \"../git-compat-util.h\"\n> +#include \"../strbuf.h\"\n> \n> /* Adapted from libiberty's basename.c. */\n> char *gitbasename (char *path)\n> @@ -25,3 +26,46 @@ char *gitbasename (char *path)\n> \t}\n> \treturn (char *)base;\n> }\n> +\n> +char *gitdirname(char *path)\n> +{\n> +\tchar *p = path, *slash = NULL, c;\n> +\tint dos_drive_prefix;\n> +\n> +\tif (!p)\n> +\t\treturn \".\";\n> +\n> +\tif ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p) {\n> +\t\tstatic struct strbuf buf = STRBUF_INIT;\n> +\n> +dot:\n> +\t\tstrbuf_reset(&buf);\n> +\t\tstrbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n\nOK, so \"Z:\" becomes \"Z:.\" (I missed the final '.' in my first reading),\nwhich sounds sensible.\n\n> +\t\treturn buf.buf;\n> +\t}\n> +\n> +\t/*\n> +\t * POSIX.1-2001 says dirname(\"/\") should return \"/\", and dirname(\"//\")\n> +\t * should return \"//\", but dirname(\"///\") should return \"/\" again.\n> +\t */\n> +\tif (is_dir_sep(*p)) {\n> +\t\tif (!p[1] || (is_dir_sep(p[1]) && !p[2]))\n> +\t\t\treturn path;\n> +\t\tslash = ++p;\n> +\t}\n> +\twhile ((c = *(p++)))\n> +\t\tif (is_dir_sep(c)) {\n> +\t\t\tchar *tentative = p - 1;\n> +\n> +\t\t\t/* POSIX.1-2001 says to ignore trailing slashes */\n> +\t\t\twhile (is_dir_sep(*p))\n> +\t\t\t\tp++;\n> +\t\t\tif (*p)\n> +\t\t\t\tslash = tentative;\n> +\t\t}\n\nOK, so we find the first slash in a run of slashes, unless that run\nof slashes is at the very end of the path. Either slash is left NUL\n(when there is no dir-sep) in which case we want to say \"current\ndirectory\", or that slash is at the end of the directory name we\nwant to return in which case we just stuff NUL and return the path.\n\nSounds sensible.\n\n> +\n> +\tif (!slash)\n> +\t\tgoto dot;\n\n;-) this is tricky. I wondered what the value of dos_drive_prefix\nat this point in my first reading, but this is correct. We must\nhave done the skip_dos_drive_prefix() thing when we came into the\nfunction already, so it is either 2 (when the original had Z: in\nfront) or 0 (for all other cases).\n\nOK.\n\n> +\t*slash = '\\0';\n> +\treturn path;\n> +}\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 0d66f3a..94f311a 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -253,6 +253,8 @@ struct itimerval {\n> #else\n> #define basename gitbasename\n> extern char *gitbasename(char *);\n> +#define dirname gitdirname\n> +extern char *gitdirname(char *);\n> #endif\n> \n> #ifndef NO_ICONV\n"},{"id":"275577","messageId":"CAPig+cRRaMbEGibYnQBTfGFQT6fybNU8e6ZAkX11V-TLAo9AfA@mail.gmail.com","threadId":"40458","inReplyTo":"c70ed05f275a44fbfae831b4cb67e59a0ce05724.1452270051.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 1/4] Refactor skipping DOS drive prefixes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-01-08T21:51:11Z","receivedAt":"2016-01-08T21:51:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jan 8, 2016 at 11:21 AM, Johannes Schindelin\n wrote:\n> Junio Hamano pointed out that there is an implicit assumption in pretty\n> much all the code calling has_dos_drive_prefix(): it assumes that the\n> DOS drive prefix is always two bytes long.\n>\n> While this assumption is pretty safe, we can still make the code more\n> readable and less error-prone by introducing a function that skips the\n> DOS drive prefix safely.\n>\n> While at it, we change the has_dos_drive_prefix() return value: it now\n> returns the number of bytes to be skipped if there is a DOS drive prefix.\n\nWith this change, code such as:\n\n for (i = has_dos_drive_prefix(src); i > 0; i--)\n ...\n\nin path.c reads a bit oddly. Renaming the function might help. For instance:\n\n for (i = dos_drive_prefix_len(src); i > 0; i--)\n ...\n\n> Signed-off-by: Johannes Schindelin \n"},{"id":"275578","messageId":"xmqq8u3z20aj.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"CAPig+cRRaMbEGibYnQBTfGFQT6fybNU8e6ZAkX11V-TLAo9AfA@mail.gmail.com","subject":"Re: [PATCH v2 1/4] Refactor skipping DOS drive prefixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-08T22:07:00Z","receivedAt":"2016-01-08T22:07:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> With this change, code such as:\n>\n> for (i = has_dos_drive_prefix(src); i > 0; i--)\n> ...\n>\n> in path.c reads a bit oddly. Renaming the function might help. For instance:\n>\n> for (i = dos_drive_prefix_len(src); i > 0; i--)\n> ...\n\nRenaming may be unnecessary churn, but I do not think we mind an\nadditional synonym, e.g.\n\n #define has_dos_drive_prefix(x) dos_drive_prefix_len(x)\n\nif some people prefer.\n"},{"id":"275594","messageId":"alpine.DEB.2.20.1601091553310.2964@virtualbox","threadId":"40458","inReplyTo":"xmqqy4bz29mp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/4] compat/basename: make basename() conform to POSIX","fromName":"Johannes Schindelin","fromEmail":"schindelin@wisc.edu","sentAt":"2016-01-09T14:53:54Z","receivedAt":"2016-01-09T14:53:54Z","isPatch":true,"sender":{"key":"schindelin@wisc.edu","avatar":null},"body":"Hi Junio,\n\nOn Fri, 8 Jan 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > According to POSIX, basename(\"/path/\") should return \"path\", not\n> > \"path/\". Likewise, basename(NULL) and basename(\"abc\") should both\n> > return \".\".\n> \n> Did you mean basename(\"abc\"), not basename(\"\"), here? \n\nI don't understand: I wrote basename(\"abc\")... ;-)\n\nCiao,\nDscho\n"},{"id":"275606","messageId":"CAPig+cRjy+xU7dZEbVfqD3LQ8YdzS2gWKL4tufSHfGSaUU-M1Q@mail.gmail.com","threadId":"40458","inReplyTo":"eca740dbf6271bd69f2ccb14163175996ef7c837.1452270051.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-01-10T04:17:58Z","receivedAt":"2016-01-10T04:17:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jan 8, 2016 at 11:21 AM, Johannes Schindelin\n wrote:\n> Unfortunately, some libgen implementations yield outcomes different from\n> what Git expects. For example, mingw-w64-crt provides a basename()\n> function, that shortens `path0/` to `path`!\n>\n> So let's verify that the basename() and dirname() functions we use conform\n> to what Git expects.\n>\n> Derived-from-code-by: Ramsay Jones \n> Signed-off-by: Johannes Schindelin \n> ---\n> diff --git a/test-path-utils.c b/test-path-utils.c\n> @@ -39,6 +39,168 @@ static void normalize_argv_string(const char **var, const char *input)\n> +struct test_data {\n> + char *from; /* input: transform from this ... */\n> + char *to; /* output: ... to this. */\n\nCan these be 'const'? If I'm reading the code correctly, I don't think\nthese values ever get passed directly to functions expecting non-const\nstrings.\n\n> +};\n> +\n> +static int test_function(struct test_data *data, char *(*func)(char *input),\n> + const char *funcname)\n> +{\n> + int failed = 0, i;\n> + static char buffer[1024];\n\nWhy is this 'static'? It is never accessed outside of this scope.\n\n> + char *to;\n> +\n> + for (i = 0; data[i].to; i++) {\n> + if (!data[i].from)\n> + to = func(NULL);\n> + else {\n> + strcpy(buffer, data[i].from);\n> + to = func(buffer);\n> + }\n> + if (strcmp(to, data[i].to)) {\n> + error(\"FAIL: %s(%s) => '%s' != '%s'\\n\",\n> + funcname, data[i].from, to, data[i].to);\n> + failed++;\n\nSince 'failed' is only ever used as a boolean, it might be clearer to say:\n\n failed = 1;\n\n> + }\n> + }\n> + return !!failed;\n\nAnd then simply:\n\n return failed;\n\n> +}\n> +\n> +static struct test_data basename_data[] = {\n> + /* --- POSIX type paths --- */\n> + { NULL, \".\" },\n\nNULL is tested here.\n\n> + { \"\", \".\" },\n> + { \".\", \".\" },\n> [...]\n> +#endif\n> + { NULL, \".\" },\n\nAnd also here. Is that intentional?\n\n> + { NULL, NULL }\n> +};\n> +\n> +static struct test_data dirname_data[] = {\n> + /* --- POSIX type paths --- */\n> + { NULL, \".\" },\n> [...]\n> +#endif\n> + { NULL, \".\" },\n\nDitto.\n\n> + { NULL, NULL }\n> +};\n"},{"id":"275632","messageId":"alpine.DEB.2.20.1601111029540.2964@virtualbox","threadId":"40458","inReplyTo":"xmqq8u3z20aj.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 1/4] Refactor skipping DOS drive prefixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-11T09:32:36Z","receivedAt":"2016-01-11T09:32:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric & Junio,\n\nOn Fri, 8 Jan 2016, Junio C Hamano wrote:\n\n> Eric Sunshine writes:\n> \n> > With this change, code such as:\n> >\n> > for (i = has_dos_drive_prefix(src); i > 0; i--)\n> > ...\n> >\n> > in path.c reads a bit oddly. Renaming the function might help. For instance:\n> >\n> > for (i = dos_drive_prefix_len(src); i > 0; i--)\n> > ...\n> \n> Renaming may be unnecessary churn, but I do not think we mind an\n> additional synonym, e.g.\n> \n> #define has_dos_drive_prefix(x) dos_drive_prefix_len(x)\n> \n> if some people prefer.\n\nI am actually not so sure about this: if I read\n`dos_drive_prefix_len(path)` I would have assumed the return value to be\n-1 if `path` does not, in fact, have a DOS drive prefix.\n\nSure, returning the length of the DOS drive prefix when just asking\nwhether it has one is a bit surprising at first, but it also makes sense:\nwe already have that information, so we might just as well use it.\n\nIn any case, I think this change (if it is really considered desirable)\ncould easily be an add-on patch by people who care about this ;-)\n\nCiao,\nDscho\n"},{"id":"275633","messageId":"alpine.DEB.2.20.1601111122410.2964@virtualbox","threadId":"40458","inReplyTo":"CAPig+cRjy+xU7dZEbVfqD3LQ8YdzS2gWKL4tufSHfGSaUU-M1Q@mail.gmail.com","subject":"Re: [PATCH v2 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-11T10:50:23Z","receivedAt":"2016-01-11T10:50:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Sat, 9 Jan 2016, Eric Sunshine wrote:\n\n> On Fri, Jan 8, 2016 at 11:21 AM, Johannes Schindelin\n> wrote:\n> > Unfortunately, some libgen implementations yield outcomes different from\n> > what Git expects. For example, mingw-w64-crt provides a basename()\n> > function, that shortens `path0/` to `path`!\n> >\n> > So let's verify that the basename() and dirname() functions we use conform\n> > to what Git expects.\n> >\n> > Derived-from-code-by: Ramsay Jones \n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > diff --git a/test-path-utils.c b/test-path-utils.c\n> > @@ -39,6 +39,168 @@ static void normalize_argv_string(const char **var, const char *input)\n> > +struct test_data {\n> > + char *from; /* input: transform from this ... */\n> > + char *to; /* output: ... to this. */\n> \n> Can these be 'const'? If I'm reading the code correctly, I don't think\n> these values ever get passed directly to functions expecting non-const\n> strings.\n\nThis, and ...\n\n> > +};\n> > +\n> > +static int test_function(struct test_data *data, char *(*func)(char *input),\n> > + const char *funcname)\n> > +{\n> > + int failed = 0, i;\n> > + static char buffer[1024];\n> \n> Why is this 'static'? It is never accessed outside of this scope.\n\n... this, and ...\n\n> > + char *to;\n> > +\n> > + for (i = 0; data[i].to; i++) {\n> > + if (!data[i].from)\n> > + to = func(NULL);\n> > + else {\n> > + strcpy(buffer, data[i].from);\n> > + to = func(buffer);\n> > + }\n> > + if (strcmp(to, data[i].to)) {\n> > + error(\"FAIL: %s(%s) => '%s' != '%s'\\n\",\n> > + funcname, data[i].from, to, data[i].to);\n> > + failed++;\n> \n> Since 'failed' is only ever used as a boolean, it might be clearer to say:\n> \n> failed = 1;\n\n... this and ...\n\n> > + }\n> > + }\n> > + return !!failed;\n> \n> And then simply:\n> \n> return failed;\n> \n> > +}\n> > +\n> > +static struct test_data basename_data[] = {\n> > + /* --- POSIX type paths --- */\n> > + { NULL, \".\" },\n> \n> NULL is tested here.\n> \n> > + { \"\", \".\" },\n> > + { \".\", \".\" },\n> > [...]\n> > +#endif\n> > + { NULL, \".\" },\n> \n> And also here. Is that intentional?\n\n... this are all valid concerns that I now addressed locally, so they will\nbe fixed in the next iteration.\n\nThanks,\nDscho\n"},{"id":"275652","messageId":"xmqqegdoyupe.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"alpine.DEB.2.20.1601111029540.2964@virtualbox","subject":"Re: [PATCH v2 1/4] Refactor skipping DOS drive prefixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-11T15:58:05Z","receivedAt":"2016-01-11T15:58:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> In any case, I think this change (if it is really considered desirable)\n> could easily be an add-on patch by people who care about this ;-)\n\nYup, in case if it was unclear, that is what I meant.\n"},{"id":"275653","messageId":"xmqqa8ocyujd.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"alpine.DEB.2.20.1601091553310.2964@virtualbox","subject":"Re: [PATCH v2 2/4] compat/basename: make basename() conform to POSIX","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-11T16:01:42Z","receivedAt":"2016-01-11T16:01:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi Junio,\n>\n> On Fri, 8 Jan 2016, Junio C Hamano wrote:\n>\n>> Johannes Schindelin writes:\n>> \n>> > According to POSIX, basename(\"/path/\") should return \"path\", not\n>> > \"path/\". Likewise, basename(NULL) and basename(\"abc\") should both\n>> > return \".\".\n>> \n>> Did you mean basename(\"abc\"), not basename(\"\"), here? \n>\n> I don't understand: I wrote basename(\"abc\")... ;-)\n\nYeah, I read that. What I didn't read was you wrote 'path' between\nslashes in the first example (the MUA was trying to render it as\n\"path\" in italic or something silly like that), and made me\nconfused: If 'path' has to go to 'path', what's so special about\n'abc' to make it go '.'?\n\nUpon second reading I noticed that slashes around the first one, but\nforgot to remove the \"why 'abc'\" comment.\n"},{"id":"275682","messageId":"cover.1452536924.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452270051.git.johannes.schindelin@gmx.de","subject":"[PATCH v3 0/4] Ensure that we can build without libgen.h","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-11T18:29:41Z","receivedAt":"2016-01-11T18:29:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This mini series adds a fall-back for the `dirname()` function that we use\ne.g. in git-am. This is necessary because not all platforms have a working\nlibgen.h.\n\nWhile at it, we ensure that our basename() drop-in conforms to the POSIX\nspecifications.\n\nIn addition to the interdiff vs v2, the commit message was fixed to\nmention basename(\"\") as cornercase (not basename(\"abc\")).\n\n\nJohannes Schindelin (4):\n Refactor skipping DOS drive prefixes\n compat/basename: make basename() conform to POSIX\n Provide a dirname() function when NO_LIBGEN_H=YesPlease\n t0060: verify that basename() and dirname() work as expected\n\n compat/basename.c | 66 ++++++++++++++++++--\n compat/mingw.c | 14 ++---\n compat/mingw.h | 10 ++-\n git-compat-util.h | 10 +++\n path.c | 14 ++---\n t/t0060-path-utils.sh | 3 +\n test-path-utils.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 7 files changed, 259 insertions(+), 24 deletions(-)\n\nInterdiff vs v2:\n\n diff --git a/test-path-utils.c b/test-path-utils.c\n index 74e74c9..4ab68ac 100644\n --- a/test-path-utils.c\n +++ b/test-path-utils.c\n @@ -40,15 +40,15 @@ static void normalize_argv_string(const char **var, const char *input)\n }\n \n struct test_data {\n -\tchar *from; /* input: transform from this ... */\n -\tchar *to; /* output: ... to this. */\n +\tconst char *from; /* input: transform from this ... */\n +\tconst char *to; /* output: ... to this. */\n };\n \n static int test_function(struct test_data *data, char *(*func)(char *input),\n \tconst char *funcname)\n {\n \tint failed = 0, i;\n -\tstatic char buffer[1024];\n +\tchar buffer[1024];\n \tchar *to;\n \n \tfor (i = 0; data[i].to; i++) {\n @@ -61,10 +61,10 @@ static int test_function(struct test_data *data, char *(*func)(char *input),\n \t\tif (strcmp(to, data[i].to)) {\n \t\t\terror(\"FAIL: %s(%s) => '%s' != '%s'\\n\",\n \t\t\t\tfuncname, data[i].from, to, data[i].to);\n -\t\t\tfailed++;\n +\t\t\tfailed = 1;\n \t\t}\n \t}\n -\treturn !!failed;\n +\treturn failed;\n }\n \n static struct test_data basename_data[] = {\n @@ -132,7 +132,6 @@ static struct test_data basename_data[] = {\n \n #endif\n #endif\n -\t{ NULL, \".\" },\n \t{ NULL, NULL }\n };\n \n @@ -197,7 +196,6 @@ static struct test_data dirname_data[] = {\n \n #endif\n #endif\n -\t{ NULL, \".\" },\n \t{ NULL, NULL }\n };\n \n\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275680","messageId":"c70ed05f275a44fbfae831b4cb67e59a0ce05724.1452536924.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452536924.git.johannes.schindelin@gmx.de","subject":"[PATCH v3 1/4] Refactor skipping DOS drive prefixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-11T18:29:47Z","receivedAt":"2016-01-11T18:29:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Junio Hamano pointed out that there is an implicit assumption in pretty\nmuch all the code calling has_dos_drive_prefix(): it assumes that the\nDOS drive prefix is always two bytes long.\n\nWhile this assumption is pretty safe, we can still make the code more\nreadable and less error-prone by introducing a function that skips the\nDOS drive prefix safely.\n\nWhile at it, we change the has_dos_drive_prefix() return value: it now\nreturns the number of bytes to be skipped if there is a DOS drive prefix.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/basename.c | 4 +---\n compat/mingw.c | 14 +++++---------\n compat/mingw.h | 10 +++++++++-\n git-compat-util.h | 8 ++++++++\n path.c | 14 +++++---------\n 5 files changed, 28 insertions(+), 22 deletions(-)\n\ndiff --git a/compat/basename.c b/compat/basename.c\nindex d8f8a3c..9f00421 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -4,9 +4,7 @@\n char *gitbasename (char *path)\n {\n \tconst char *base;\n-\t/* Skip over the disk name in MSDOS pathnames. */\n-\tif (has_dos_drive_prefix(path))\n-\t\tpath += 2;\n+\tskip_dos_drive_prefix(&path);\n \tfor (base = path; *path; path++) {\n \t\tif (is_dir_sep(*path))\n \t\t\tbase = path + 1;\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 5edea29..1b3530a 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1934,26 +1934,22 @@ pid_t waitpid(pid_t pid, int *status, int options)\n \n int mingw_offset_1st_component(const char *path)\n {\n-\tint offset = 0;\n-\tif (has_dos_drive_prefix(path))\n-\t\toffset = 2;\n+\tchar *pos = (char *)path;\n \n \t/* unc paths */\n-\telse if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {\n-\n+\tif (!skip_dos_drive_prefix(&pos) &&\n+\t\t\tis_dir_sep(pos[0]) && is_dir_sep(pos[1])) {\n \t\t/* skip server name */\n-\t\tchar *pos = strpbrk(path + 2, \"\\\\/\");\n+\t\tpos = strpbrk(pos + 2, \"\\\\/\");\n \t\tif (!pos)\n \t\t\treturn 0; /* Error: malformed unc path */\n \n \t\tdo {\n \t\t\tpos++;\n \t\t} while (*pos && !is_dir_sep(*pos));\n-\n-\t\toffset = pos - path;\n \t}\n \n-\treturn offset + is_dir_sep(path[offset]);\n+\treturn pos + is_dir_sep(*pos) - path;\n }\n \n int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 57ca477..b3e5044 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -361,7 +361,15 @@ HANDLE winansi_get_osfhandle(int fd);\n * git specific compatibility\n */\n \n-#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')\n+#define has_dos_drive_prefix(path) \\\n+\t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n+static inline int mingw_skip_dos_drive_prefix(char **path)\n+{\n+\tint ret = has_dos_drive_prefix(*path);\n+\t*path += ret;\n+\treturn ret;\n+}\n+#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n static inline char *mingw_find_last_dir_sep(const char *path)\n {\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 2da0a75..0d66f3a 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -335,6 +335,14 @@ static inline int git_has_dos_drive_prefix(const char *path)\n #define has_dos_drive_prefix git_has_dos_drive_prefix\n #endif\n \n+#ifndef skip_dos_drive_prefix\n+static inline int git_skip_dos_drive_prefix(const char **path)\n+{\n+\treturn 0;\n+}\n+#define skip_dos_drive_prefix git_skip_dos_drive_prefix\n+#endif\n+\n #ifndef is_dir_sep\n static inline int git_is_dir_sep(int c)\n {\ndiff --git a/path.c b/path.c\nindex 3cd155e..8b7e168 100644\n--- a/path.c\n+++ b/path.c\n@@ -782,13 +782,10 @@ const char *relative_path(const char *in, const char *prefix,\n \telse if (!prefix_len)\n \t\treturn in;\n \n-\tif (have_same_root(in, prefix)) {\n+\tif (have_same_root(in, prefix))\n \t\t/* bypass dos_drive, for \"c:\" is identical to \"C:\" */\n-\t\tif (has_dos_drive_prefix(in)) {\n-\t\t\ti = 2;\n-\t\t\tj = 2;\n-\t\t}\n-\t} else {\n+\t\ti = j = has_dos_drive_prefix(in);\n+\telse {\n \t\treturn in;\n \t}\n \n@@ -943,11 +940,10 @@ const char *remove_leading_path(const char *in, const char *prefix)\n int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n {\n \tchar *dst0;\n+\tint i;\n \n-\tif (has_dos_drive_prefix(src)) {\n+\tfor (i = has_dos_drive_prefix(src); i > 0; i--)\n \t\t*dst++ = *src++;\n-\t\t*dst++ = *src++;\n-\t}\n \tdst0 = dst;\n \n \tif (is_dir_sep(*src)) {\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275681","messageId":"00c89c7a9fdadbc1631715af7f84b38448fdd315.1452536924.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452536924.git.johannes.schindelin@gmx.de","subject":"[PATCH v3 2/4] compat/basename: make basename() conform to POSIX","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-11T18:29:54Z","receivedAt":"2016-01-11T18:29:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"According to POSIX, basename(\"/path/\") should return \"path\", not\n\"path/\". Likewise, basename(NULL) and basename(\"\") should both\nreturn \".\" to conform.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/basename.c | 20 +++++++++++++++++---\n 1 file changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/compat/basename.c b/compat/basename.c\nindex 9f00421..0f1b0b0 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -4,10 +4,24 @@\n char *gitbasename (char *path)\n {\n \tconst char *base;\n-\tskip_dos_drive_prefix(&path);\n+\n+\tif (path)\n+\t\tskip_dos_drive_prefix(&path);\n+\n+\tif (!path || !*path)\n+\t\treturn \".\";\n+\n \tfor (base = path; *path; path++) {\n-\t\tif (is_dir_sep(*path))\n-\t\t\tbase = path + 1;\n+\t\tif (!is_dir_sep(*path))\n+\t\t\tcontinue;\n+\t\tdo {\n+\t\t\tpath++;\n+\t\t} while (is_dir_sep(*path));\n+\t\tif (*path)\n+\t\t\tbase = path;\n+\t\telse\n+\t\t\twhile (--path != base && is_dir_sep(*path))\n+\t\t\t\t*path = '\\0';\n \t}\n \treturn (char *)base;\n }\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275684","messageId":"0bab11634c8f05751b2ed5879bc4100441bba4b9.1452536924.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452536924.git.johannes.schindelin@gmx.de","subject":"[PATCH v3 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-11T18:30:00Z","receivedAt":"2016-01-11T18:30:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When there is no `libgen.h` to our disposal, we miss the `dirname()`\nfunction.\n\nSo far, we only had one user of that function: credential-cache--daemon\n(which was only compiled when Unix sockets are available, anyway). But\nnow we also have `builtin/am.c` as user, so we need it.\n\nSince `dirname()` is a sibling of `basename()`, we simply put our very\nown `gitdirname()` implementation next to `gitbasename()` and use it\nif `NO_LIBGEN_H` has been set.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/basename.c | 44 ++++++++++++++++++++++++++++++++++++++++++++\n git-compat-util.h | 2 ++\n 2 files changed, 46 insertions(+)\n\ndiff --git a/compat/basename.c b/compat/basename.c\nindex 0f1b0b0..0a2ed25 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -1,4 +1,5 @@\n #include \"../git-compat-util.h\"\n+#include \"../strbuf.h\"\n \n /* Adapted from libiberty's basename.c. */\n char *gitbasename (char *path)\n@@ -25,3 +26,46 @@ char *gitbasename (char *path)\n \t}\n \treturn (char *)base;\n }\n+\n+char *gitdirname(char *path)\n+{\n+\tchar *p = path, *slash = NULL, c;\n+\tint dos_drive_prefix;\n+\n+\tif (!p)\n+\t\treturn \".\";\n+\n+\tif ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p) {\n+\t\tstatic struct strbuf buf = STRBUF_INIT;\n+\n+dot:\n+\t\tstrbuf_reset(&buf);\n+\t\tstrbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n+\t\treturn buf.buf;\n+\t}\n+\n+\t/*\n+\t * POSIX.1-2001 says dirname(\"/\") should return \"/\", and dirname(\"//\")\n+\t * should return \"//\", but dirname(\"///\") should return \"/\" again.\n+\t */\n+\tif (is_dir_sep(*p)) {\n+\t\tif (!p[1] || (is_dir_sep(p[1]) && !p[2]))\n+\t\t\treturn path;\n+\t\tslash = ++p;\n+\t}\n+\twhile ((c = *(p++)))\n+\t\tif (is_dir_sep(c)) {\n+\t\t\tchar *tentative = p - 1;\n+\n+\t\t\t/* POSIX.1-2001 says to ignore trailing slashes */\n+\t\t\twhile (is_dir_sep(*p))\n+\t\t\t\tp++;\n+\t\t\tif (*p)\n+\t\t\t\tslash = tentative;\n+\t\t}\n+\n+\tif (!slash)\n+\t\tgoto dot;\n+\t*slash = '\\0';\n+\treturn path;\n+}\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 0d66f3a..94f311a 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -253,6 +253,8 @@ struct itimerval {\n #else\n #define basename gitbasename\n extern char *gitbasename(char *);\n+#define dirname gitdirname\n+extern char *gitdirname(char *);\n #endif\n \n #ifndef NO_ICONV\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275683","messageId":"056411e53cd5772f36ce4e0c2f18aae0db015970.1452536924.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452536924.git.johannes.schindelin@gmx.de","subject":"[PATCH v3 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-11T18:30:04Z","receivedAt":"2016-01-11T18:30:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Unfortunately, some libgen implementations yield outcomes different\nfrom what Git expects. For example, mingw-w64-crt provides a basename()\nfunction, that shortens `path0/` to `path`!\n\nSo let's verify that the basename() and dirname() functions we use\nconform to what Git expects.\n\nDerived-from-code-by: Ramsay Jones \nSigned-off-by: Johannes Schindelin \n---\n t/t0060-path-utils.sh | 3 +\n test-path-utils.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 169 insertions(+)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 627ef85..f0152a7 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -59,6 +59,9 @@ case $(uname -s) in\n \t;;\n esac\n \n+test_expect_success basename 'test-path-utils basename'\n+test_expect_success dirname 'test-path-utils dirname'\n+\n norm_path \"\" \"\"\n norm_path . \"\"\n norm_path ./ \"\"\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex c67bf65..4ab68ac 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -39,6 +39,166 @@ static void normalize_argv_string(const char **var, const char *input)\n \t\tdie(\"Bad value: %s\\n\", input);\n }\n \n+struct test_data {\n+\tconst char *from; /* input: transform from this ... */\n+\tconst char *to; /* output: ... to this. */\n+};\n+\n+static int test_function(struct test_data *data, char *(*func)(char *input),\n+\tconst char *funcname)\n+{\n+\tint failed = 0, i;\n+\tchar buffer[1024];\n+\tchar *to;\n+\n+\tfor (i = 0; data[i].to; i++) {\n+\t\tif (!data[i].from)\n+\t\t\tto = func(NULL);\n+\t\telse {\n+\t\t\tstrcpy(buffer, data[i].from);\n+\t\t\tto = func(buffer);\n+\t\t}\n+\t\tif (strcmp(to, data[i].to)) {\n+\t\t\terror(\"FAIL: %s(%s) => '%s' != '%s'\\n\",\n+\t\t\t\tfuncname, data[i].from, to, data[i].to);\n+\t\t\tfailed = 1;\n+\t\t}\n+\t}\n+\treturn failed;\n+}\n+\n+static struct test_data basename_data[] = {\n+\t/* --- POSIX type paths --- */\n+\t{ NULL, \".\" },\n+\t{ \"\", \".\" },\n+\t{ \".\", \".\" },\n+\t{ \"..\", \"..\" },\n+\t{ \"/\", \"/\" },\n+#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n+\t{ \"//\", \"//\" },\n+\t{ \"///\", \"//\" },\n+\t{ \"////\", \"//\" },\n+#else\n+\t{ \"//\", \"/\" },\n+\t{ \"///\", \"/\" },\n+\t{ \"////\", \"/\" },\n+#endif\n+\t{ \"usr\", \"usr\" },\n+\t{ \"/usr\", \"usr\" },\n+\t{ \"/usr/\", \"usr\" },\n+\t{ \"/usr//\", \"usr\" },\n+\t{ \"/usr/lib\", \"lib\" },\n+\t{ \"usr/lib\", \"lib\" },\n+\t{ \"usr/lib///\", \"lib\" },\n+\n+#if defined(__MINGW32__) || defined(_MSC_VER)\n+\n+\t/* --- win32 type paths --- */\n+\t{ \"\\\\usr\", \"usr\" },\n+\t{ \"\\\\usr\\\\\", \"usr\" },\n+\t{ \"\\\\usr\\\\\\\\\", \"usr\" },\n+\t{ \"\\\\usr\\\\lib\", \"lib\" },\n+\t{ \"usr\\\\lib\", \"lib\" },\n+\t{ \"usr\\\\lib\\\\\\\\\\\\\", \"lib\" },\n+\t{ \"C:/usr\", \"usr\" },\n+\t{ \"C:/usr\", \"usr\" },\n+\t{ \"C:/usr/\", \"usr\" },\n+\t{ \"C:/usr//\", \"usr\" },\n+\t{ \"C:/usr/lib\", \"lib\" },\n+\t{ \"C:usr/lib\", \"lib\" },\n+\t{ \"C:usr/lib///\", \"lib\" },\n+\t{ \"C:\", \".\" },\n+\t{ \"C:a\", \"a\" },\n+\t{ \"C:/\", \"/\" },\n+\t{ \"C:///\", \"/\" },\n+#if defined(NO_LIBGEN_H)\n+\t{ \"\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n+#else\n+\n+\t/* win32 platform variations: */\n+#if defined(__MINGW32__)\n+\t{ \"\\\\\", \"/\" },\n+\t{ \"\\\\\\\\\", \"/\" },\n+\t{ \"\\\\\\\\\\\\\", \"/\" },\n+#endif\n+\n+#if defined(_MSC_VER)\n+\t{ \"\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n+#endif\n+\n+#endif\n+#endif\n+\t{ NULL, NULL }\n+};\n+\n+static struct test_data dirname_data[] = {\n+\t/* --- POSIX type paths --- */\n+\t{ NULL, \".\" },\n+\t{ \"\", \".\" },\n+\t{ \".\", \".\" },\n+\t{ \"..\", \".\" },\n+\t{ \"/\", \"/\" },\n+\t{ \"//\", \"//\" },\n+#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n+\t{ \"///\", \"//\" },\n+\t{ \"////\", \"//\" },\n+#else\n+\t{ \"///\", \"/\" },\n+\t{ \"////\", \"/\" },\n+#endif\n+\t{ \"usr\", \".\" },\n+\t{ \"/usr\", \"/\" },\n+\t{ \"/usr/\", \"/\" },\n+\t{ \"/usr//\", \"/\" },\n+\t{ \"/usr/lib\", \"/usr\" },\n+\t{ \"usr/lib\", \"usr\" },\n+\t{ \"usr/lib///\", \"usr\" },\n+\n+#if defined(__MINGW32__) || defined(_MSC_VER)\n+\n+\t/* --- win32 type paths --- */\n+\t{ \"\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\", \"\\\\\\\\\" },\n+\t{ \"\\\\usr\", \"\\\\\" },\n+\t{ \"\\\\usr\\\\\", \"\\\\\" },\n+\t{ \"\\\\usr\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\usr\\\\lib\", \"\\\\usr\" },\n+\t{ \"usr\\\\lib\", \"usr\" },\n+\t{ \"usr\\\\lib\\\\\\\\\\\\\", \"usr\" },\n+\t{ \"C:a\", \"C:.\" },\n+\t{ \"C:/\", \"C:/\" },\n+\t{ \"C:///\", \"C:/\" },\n+\t{ \"C:/usr\", \"C:/\" },\n+\t{ \"C:/usr/\", \"C:/\" },\n+\t{ \"C:/usr//\", \"C:/\" },\n+\t{ \"C:/usr/lib\", \"C:/usr\" },\n+\t{ \"C:usr/lib\", \"C:usr\" },\n+\t{ \"C:usr/lib///\", \"C:usr\" },\n+\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\\\\\\\\\", \"\\\\\" },\n+#if defined(NO_LIBGEN_H)\n+\t{ \"C:\", \"C:.\" },\n+#else\n+\n+\t/* win32 platform variations: */\n+#if defined(__MINGW32__)\n+\t/* the following is clearly wrong ... */\n+\t{ \"C:\", \".\" },\n+#endif\n+\n+#if defined(_MSC_VER)\n+\t{ \"C:\", \"C:.\" },\n+#endif\n+\n+#endif\n+#endif\n+\t{ NULL, NULL }\n+};\n+\n int main(int argc, char **argv)\n {\n \tif (argc == 3 && !strcmp(argv[1], \"normalize_path_copy\")) {\n@@ -133,6 +293,12 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n \n+\tif (argc == 2 && !strcmp(argv[1], \"basename\"))\n+\t\treturn test_function(basename_data, basename, argv[1]);\n+\n+\tif (argc == 2 && !strcmp(argv[1], \"dirname\"))\n+\t\treturn test_function(dirname_data, dirname, argv[1]);\n+\n \tfprintf(stderr, \"%s: unknown function name: %s\\n\", argv[0],\n \t\targv[1] ? argv[1] : \"(there was none)\");\n \treturn 1;\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275704","messageId":"CAPig+cQmhc=DmptyYWy1p3z4rz7_h-3XRrtFH7XxoW77z5Mz-A@mail.gmail.com","threadId":"40458","inReplyTo":"0bab11634c8f05751b2ed5879bc4100441bba4b9.1452536924.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v3 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-01-11T20:33:49Z","receivedAt":"2016-01-11T20:33:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jan 11, 2016 at 1:30 PM, Johannes Schindelin\n wrote:\n> When there is no `libgen.h` to our disposal, we miss the `dirname()`\n> function.\n>\n> So far, we only had one user of that function: credential-cache--daemon\n> (which was only compiled when Unix sockets are available, anyway). But\n> now we also have `builtin/am.c` as user, so we need it.\n>\n> Since `dirname()` is a sibling of `basename()`, we simply put our very\n> own `gitdirname()` implementation next to `gitbasename()` and use it\n> if `NO_LIBGEN_H` has been set.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> diff --git a/compat/basename.c b/compat/basename.c\n> @@ -25,3 +26,46 @@ char *gitbasename (char *path)\n> +char *gitdirname(char *path)\n> +{\n> + char *p = path, *slash = NULL, c;\n> + int dos_drive_prefix;\n> +\n> + if (!p)\n> + return \".\";\n> +\n> + if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p) {\n> + static struct strbuf buf = STRBUF_INIT;\n> +\n> +dot:\n> + strbuf_reset(&buf);\n> + strbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n> + return buf.buf;\n> + }\n> +\n> + /*\n> + * POSIX.1-2001 says dirname(\"/\") should return \"/\", and dirname(\"//\")\n> + * should return \"//\", but dirname(\"///\") should return \"/\" again.\n> + */\n> + if (is_dir_sep(*p)) {\n> + if (!p[1] || (is_dir_sep(p[1]) && !p[2]))\n> + return path;\n> + slash = ++p;\n> + }\n> + while ((c = *(p++)))\n> + if (is_dir_sep(c)) {\n> + char *tentative = p - 1;\n> +\n> + /* POSIX.1-2001 says to ignore trailing slashes */\n> + while (is_dir_sep(*p))\n> + p++;\n> + if (*p)\n> + slash = tentative;\n> + }\n> +\n> + if (!slash)\n> + goto dot;\n> + *slash = '\\0';\n> + return path;\n> +}\n\nI wonder if this would be a bit easier to follow if it was structured\nsomething like this:\n\n static struct strbuf buf = STRBUF_INIT;\n\n if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p)\n goto dot;\n\n ...\n if (is_dir_sep(*p)) {\n ...\n }\n ...\n while ((c = *(p++)))\n ...\n\n if (slash) {\n *slash = '\\0';\n return path;\n }\n\n dot:\n strbuf_reset(&buf);\n strbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n return buf.buf;\n"},{"id":"275722","messageId":"xmqqtwmjvi75.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"CAPig+cQmhc=DmptyYWy1p3z4rz7_h-3XRrtFH7XxoW77z5Mz-A@mail.gmail.com","subject":"Re: [PATCH v3 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-11T22:56:30Z","receivedAt":"2016-01-11T22:56:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> I wonder if this would be a bit easier to follow if it was structured\n> something like this:\n>\n> static struct strbuf buf = STRBUF_INIT;\n>\n> if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p)\n> goto dot;\n>\n> ...\n> if (is_dir_sep(*p)) {\n> ...\n> }\n> ...\n> while ((c = *(p++)))\n> ...\n>\n> if (slash) {\n> *slash = '\\0';\n> return path;\n> }\n>\n> dot:\n> strbuf_reset(&buf);\n> strbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n> return buf.buf;\n\nI'll queue the one from Dscho as-is for today, but avoiding the\n\"jump back to a place where it happens to have an identical clean-up\nthat need to happen\" and defining the clean-up path at the end like\nthis would probably be easier to follow. It certainly would have\nsaved one comment in the previous review cycle from me.\n\nThanks.\n"},{"id":"275723","messageId":"xmqqpox7vi1t.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"cover.1452536924.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v3 0/4] Ensure that we can build without libgen.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-11T22:59:42Z","receivedAt":"2016-01-11T22:59:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks. Queued but I am OK if you agree it is better to replace 3/4\nwith Eric's.\n"},{"id":"275760","messageId":"alpine.DEB.2.20.1601120856270.2964@virtualbox","threadId":"40458","inReplyTo":"xmqqtwmjvi75.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-12T07:57:05Z","receivedAt":"2016-01-12T07:57:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric & Junio,\n\nOn Mon, 11 Jan 2016, Junio C Hamano wrote:\n\n> Eric Sunshine writes:\n> \n> > I wonder if this would be a bit easier to follow if it was structured\n> > something like this:\n> >\n> > static struct strbuf buf = STRBUF_INIT;\n> >\n> > if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p)\n> > goto dot;\n> >\n> > ...\n> > if (is_dir_sep(*p)) {\n> > ...\n> > }\n> > ...\n> > while ((c = *(p++)))\n> > ...\n> >\n> > if (slash) {\n> > *slash = '\\0';\n> > return path;\n> > }\n> >\n> > dot:\n> > strbuf_reset(&buf);\n> > strbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n> > return buf.buf;\n> \n> I'll queue the one from Dscho as-is for today, but avoiding the\n> \"jump back to a place where it happens to have an identical clean-up\n> that need to happen\" and defining the clean-up path at the end like\n> this would probably be easier to follow. It certainly would have\n> saved one comment in the previous review cycle from me.\n\nThanks! I changed it and will mail out v4 shortly.\n\nCiao,\nDscho\n"},{"id":"275763","messageId":"cover.1452585382.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452536924.git.johannes.schindelin@gmx.de","subject":"[PATCH v4 0/4] Ensure that we can build without libgen.h","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-12T07:57:16Z","receivedAt":"2016-01-12T07:57:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This mini series adds a fall-back for the `dirname()` function that we use\ne.g. in git-am. This is necessary because not all platforms have a working\nlibgen.h.\n\nWhile at it, we ensure that our basename() drop-in conforms to the POSIX\nspecifications.\n\nIn addition to Eric's style improvement, v4 also fixes the signature\nof skip_dos_drive_prefix() in the non-Windows case.\n\n\nJohannes Schindelin (4):\n Refactor skipping DOS drive prefixes\n compat/basename: make basename() conform to POSIX\n Provide a dirname() function when NO_LIBGEN_H=YesPlease\n t0060: verify that basename() and dirname() work as expected\n\n compat/basename.c | 66 ++++++++++++++++++--\n compat/mingw.c | 14 ++---\n compat/mingw.h | 10 ++-\n git-compat-util.h | 10 +++\n path.c | 14 ++---\n t/t0060-path-utils.sh | 3 +\n test-path-utils.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 7 files changed, 259 insertions(+), 24 deletions(-)\n\nInterdiff vs v3:\n\n diff --git a/compat/basename.c b/compat/basename.c\n index 0a2ed25..96bd953 100644\n --- a/compat/basename.c\n +++ b/compat/basename.c\n @@ -29,20 +29,15 @@ char *gitbasename (char *path)\n \n char *gitdirname(char *path)\n {\n +\tstatic struct strbuf buf = STRBUF_INIT;\n \tchar *p = path, *slash = NULL, c;\n \tint dos_drive_prefix;\n \n \tif (!p)\n \t\treturn \".\";\n \n -\tif ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p) {\n -\t\tstatic struct strbuf buf = STRBUF_INIT;\n -\n -dot:\n -\t\tstrbuf_reset(&buf);\n -\t\tstrbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n -\t\treturn buf.buf;\n -\t}\n +\tif ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p)\n +\t\tgoto dot;\n \n \t/*\n \t * POSIX.1-2001 says dirname(\"/\") should return \"/\", and dirname(\"//\")\n @@ -64,8 +59,13 @@ dot:\n \t\t\t\tslash = tentative;\n \t\t}\n \n -\tif (!slash)\n -\t\tgoto dot;\n -\t*slash = '\\0';\n -\treturn path;\n +\tif (slash) {\n +\t\t*slash = '\\0';\n +\t\treturn path;\n +\t}\n +\n +dot:\n +\tstrbuf_reset(&buf);\n +\tstrbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n +\treturn buf.buf;\n }\n diff --git a/git-compat-util.h b/git-compat-util.h\n index 94f311a..5f72f1c 100644\n --- a/git-compat-util.h\n +++ b/git-compat-util.h\n @@ -338,7 +338,7 @@ static inline int git_has_dos_drive_prefix(const char *path)\n #endif\n \n #ifndef skip_dos_drive_prefix\n -static inline int git_skip_dos_drive_prefix(const char **path)\n +static inline int git_skip_dos_drive_prefix(char **path)\n {\n \treturn 0;\n }\n\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275761","messageId":"05cb9e00756e8a364f972cd227804764f6a6380c.1452585382.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452585382.git.johannes.schindelin@gmx.de","subject":"[PATCH v4 1/4] Refactor skipping DOS drive prefixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-12T07:57:22Z","receivedAt":"2016-01-12T07:57:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Junio Hamano pointed out that there is an implicit assumption in pretty\nmuch all the code calling has_dos_drive_prefix(): it assumes that the\nDOS drive prefix is always two bytes long.\n\nWhile this assumption is pretty safe, we can still make the code more\nreadable and less error-prone by introducing a function that skips the\nDOS drive prefix safely.\n\nWhile at it, we change the has_dos_drive_prefix() return value: it now\nreturns the number of bytes to be skipped if there is a DOS drive prefix.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/basename.c | 4 +---\n compat/mingw.c | 14 +++++---------\n compat/mingw.h | 10 +++++++++-\n git-compat-util.h | 8 ++++++++\n path.c | 14 +++++---------\n 5 files changed, 28 insertions(+), 22 deletions(-)\n\ndiff --git a/compat/basename.c b/compat/basename.c\nindex d8f8a3c..9f00421 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -4,9 +4,7 @@\n char *gitbasename (char *path)\n {\n \tconst char *base;\n-\t/* Skip over the disk name in MSDOS pathnames. */\n-\tif (has_dos_drive_prefix(path))\n-\t\tpath += 2;\n+\tskip_dos_drive_prefix(&path);\n \tfor (base = path; *path; path++) {\n \t\tif (is_dir_sep(*path))\n \t\t\tbase = path + 1;\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 5edea29..1b3530a 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1934,26 +1934,22 @@ pid_t waitpid(pid_t pid, int *status, int options)\n \n int mingw_offset_1st_component(const char *path)\n {\n-\tint offset = 0;\n-\tif (has_dos_drive_prefix(path))\n-\t\toffset = 2;\n+\tchar *pos = (char *)path;\n \n \t/* unc paths */\n-\telse if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {\n-\n+\tif (!skip_dos_drive_prefix(&pos) &&\n+\t\t\tis_dir_sep(pos[0]) && is_dir_sep(pos[1])) {\n \t\t/* skip server name */\n-\t\tchar *pos = strpbrk(path + 2, \"\\\\/\");\n+\t\tpos = strpbrk(pos + 2, \"\\\\/\");\n \t\tif (!pos)\n \t\t\treturn 0; /* Error: malformed unc path */\n \n \t\tdo {\n \t\t\tpos++;\n \t\t} while (*pos && !is_dir_sep(*pos));\n-\n-\t\toffset = pos - path;\n \t}\n \n-\treturn offset + is_dir_sep(path[offset]);\n+\treturn pos + is_dir_sep(*pos) - path;\n }\n \n int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 57ca477..b3e5044 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -361,7 +361,15 @@ HANDLE winansi_get_osfhandle(int fd);\n * git specific compatibility\n */\n \n-#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')\n+#define has_dos_drive_prefix(path) \\\n+\t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n+static inline int mingw_skip_dos_drive_prefix(char **path)\n+{\n+\tint ret = has_dos_drive_prefix(*path);\n+\t*path += ret;\n+\treturn ret;\n+}\n+#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n static inline char *mingw_find_last_dir_sep(const char *path)\n {\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 2da0a75..fbb11bb 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -335,6 +335,14 @@ static inline int git_has_dos_drive_prefix(const char *path)\n #define has_dos_drive_prefix git_has_dos_drive_prefix\n #endif\n \n+#ifndef skip_dos_drive_prefix\n+static inline int git_skip_dos_drive_prefix(char **path)\n+{\n+\treturn 0;\n+}\n+#define skip_dos_drive_prefix git_skip_dos_drive_prefix\n+#endif\n+\n #ifndef is_dir_sep\n static inline int git_is_dir_sep(int c)\n {\ndiff --git a/path.c b/path.c\nindex 3cd155e..8b7e168 100644\n--- a/path.c\n+++ b/path.c\n@@ -782,13 +782,10 @@ const char *relative_path(const char *in, const char *prefix,\n \telse if (!prefix_len)\n \t\treturn in;\n \n-\tif (have_same_root(in, prefix)) {\n+\tif (have_same_root(in, prefix))\n \t\t/* bypass dos_drive, for \"c:\" is identical to \"C:\" */\n-\t\tif (has_dos_drive_prefix(in)) {\n-\t\t\ti = 2;\n-\t\t\tj = 2;\n-\t\t}\n-\t} else {\n+\t\ti = j = has_dos_drive_prefix(in);\n+\telse {\n \t\treturn in;\n \t}\n \n@@ -943,11 +940,10 @@ const char *remove_leading_path(const char *in, const char *prefix)\n int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n {\n \tchar *dst0;\n+\tint i;\n \n-\tif (has_dos_drive_prefix(src)) {\n+\tfor (i = has_dos_drive_prefix(src); i > 0; i--)\n \t\t*dst++ = *src++;\n-\t\t*dst++ = *src++;\n-\t}\n \tdst0 = dst;\n \n \tif (is_dir_sep(*src)) {\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275762","messageId":"a7375faaba405354b30bc19c6edbdb1ef7c68ab1.1452585382.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452585382.git.johannes.schindelin@gmx.de","subject":"[PATCH v4 2/4] compat/basename: make basename() conform to POSIX","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-12T07:57:30Z","receivedAt":"2016-01-12T07:57:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"According to POSIX, basename(\"/path/\") should return \"path\", not\n\"path/\". Likewise, basename(NULL) and basename(\"\") should both\nreturn \".\" to conform.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/basename.c | 20 +++++++++++++++++---\n 1 file changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/compat/basename.c b/compat/basename.c\nindex 9f00421..0f1b0b0 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -4,10 +4,24 @@\n char *gitbasename (char *path)\n {\n \tconst char *base;\n-\tskip_dos_drive_prefix(&path);\n+\n+\tif (path)\n+\t\tskip_dos_drive_prefix(&path);\n+\n+\tif (!path || !*path)\n+\t\treturn \".\";\n+\n \tfor (base = path; *path; path++) {\n-\t\tif (is_dir_sep(*path))\n-\t\t\tbase = path + 1;\n+\t\tif (!is_dir_sep(*path))\n+\t\t\tcontinue;\n+\t\tdo {\n+\t\t\tpath++;\n+\t\t} while (is_dir_sep(*path));\n+\t\tif (*path)\n+\t\t\tbase = path;\n+\t\telse\n+\t\t\twhile (--path != base && is_dir_sep(*path))\n+\t\t\t\t*path = '\\0';\n \t}\n \treturn (char *)base;\n }\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275764","messageId":"04a7a497f9a5771d4dbf5fd605f138607b2bae0a.1452585382.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452585382.git.johannes.schindelin@gmx.de","subject":"[PATCH v4 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-12T07:57:36Z","receivedAt":"2016-01-12T07:57:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When there is no `libgen.h` to our disposal, we miss the `dirname()`\nfunction.\n\nSo far, we only had one user of that function: credential-cache--daemon\n(which was only compiled when Unix sockets are available, anyway). But\nnow we also have `builtin/am.c` as user, so we need it.\n\nSince `dirname()` is a sibling of `basename()`, we simply put our very\nown `gitdirname()` implementation next to `gitbasename()` and use it\nif `NO_LIBGEN_H` has been set.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/basename.c | 44 ++++++++++++++++++++++++++++++++++++++++++++\n git-compat-util.h | 2 ++\n 2 files changed, 46 insertions(+)\n\ndiff --git a/compat/basename.c b/compat/basename.c\nindex 0f1b0b0..96bd953 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -1,4 +1,5 @@\n #include \"../git-compat-util.h\"\n+#include \"../strbuf.h\"\n \n /* Adapted from libiberty's basename.c. */\n char *gitbasename (char *path)\n@@ -25,3 +26,46 @@ char *gitbasename (char *path)\n \t}\n \treturn (char *)base;\n }\n+\n+char *gitdirname(char *path)\n+{\n+\tstatic struct strbuf buf = STRBUF_INIT;\n+\tchar *p = path, *slash = NULL, c;\n+\tint dos_drive_prefix;\n+\n+\tif (!p)\n+\t\treturn \".\";\n+\n+\tif ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p)\n+\t\tgoto dot;\n+\n+\t/*\n+\t * POSIX.1-2001 says dirname(\"/\") should return \"/\", and dirname(\"//\")\n+\t * should return \"//\", but dirname(\"///\") should return \"/\" again.\n+\t */\n+\tif (is_dir_sep(*p)) {\n+\t\tif (!p[1] || (is_dir_sep(p[1]) && !p[2]))\n+\t\t\treturn path;\n+\t\tslash = ++p;\n+\t}\n+\twhile ((c = *(p++)))\n+\t\tif (is_dir_sep(c)) {\n+\t\t\tchar *tentative = p - 1;\n+\n+\t\t\t/* POSIX.1-2001 says to ignore trailing slashes */\n+\t\t\twhile (is_dir_sep(*p))\n+\t\t\t\tp++;\n+\t\t\tif (*p)\n+\t\t\t\tslash = tentative;\n+\t\t}\n+\n+\tif (slash) {\n+\t\t*slash = '\\0';\n+\t\treturn path;\n+\t}\n+\n+dot:\n+\tstrbuf_reset(&buf);\n+\tstrbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n+\treturn buf.buf;\n+}\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex fbb11bb..5f72f1c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -253,6 +253,8 @@ struct itimerval {\n #else\n #define basename gitbasename\n extern char *gitbasename(char *);\n+#define dirname gitdirname\n+extern char *gitdirname(char *);\n #endif\n \n #ifndef NO_ICONV\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275765","messageId":"7d73267984ab029df022477e341c536e111eafdd.1452585382.git.johannes.schindelin@gmx.de","threadId":"40458","inReplyTo":"cover.1452585382.git.johannes.schindelin@gmx.de","subject":"[PATCH v4 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-12T07:57:57Z","receivedAt":"2016-01-12T07:57:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Unfortunately, some libgen implementations yield outcomes different\nfrom what Git expects. For example, mingw-w64-crt provides a basename()\nfunction, that shortens `path0/` to `path`!\n\nSo let's verify that the basename() and dirname() functions we use\nconform to what Git expects.\n\nDerived-from-code-by: Ramsay Jones \nSigned-off-by: Johannes Schindelin \n---\n t/t0060-path-utils.sh | 3 +\n test-path-utils.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 169 insertions(+)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 627ef85..f0152a7 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -59,6 +59,9 @@ case $(uname -s) in\n \t;;\n esac\n \n+test_expect_success basename 'test-path-utils basename'\n+test_expect_success dirname 'test-path-utils dirname'\n+\n norm_path \"\" \"\"\n norm_path . \"\"\n norm_path ./ \"\"\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex c67bf65..4ab68ac 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -39,6 +39,166 @@ static void normalize_argv_string(const char **var, const char *input)\n \t\tdie(\"Bad value: %s\\n\", input);\n }\n \n+struct test_data {\n+\tconst char *from; /* input: transform from this ... */\n+\tconst char *to; /* output: ... to this. */\n+};\n+\n+static int test_function(struct test_data *data, char *(*func)(char *input),\n+\tconst char *funcname)\n+{\n+\tint failed = 0, i;\n+\tchar buffer[1024];\n+\tchar *to;\n+\n+\tfor (i = 0; data[i].to; i++) {\n+\t\tif (!data[i].from)\n+\t\t\tto = func(NULL);\n+\t\telse {\n+\t\t\tstrcpy(buffer, data[i].from);\n+\t\t\tto = func(buffer);\n+\t\t}\n+\t\tif (strcmp(to, data[i].to)) {\n+\t\t\terror(\"FAIL: %s(%s) => '%s' != '%s'\\n\",\n+\t\t\t\tfuncname, data[i].from, to, data[i].to);\n+\t\t\tfailed = 1;\n+\t\t}\n+\t}\n+\treturn failed;\n+}\n+\n+static struct test_data basename_data[] = {\n+\t/* --- POSIX type paths --- */\n+\t{ NULL, \".\" },\n+\t{ \"\", \".\" },\n+\t{ \".\", \".\" },\n+\t{ \"..\", \"..\" },\n+\t{ \"/\", \"/\" },\n+#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n+\t{ \"//\", \"//\" },\n+\t{ \"///\", \"//\" },\n+\t{ \"////\", \"//\" },\n+#else\n+\t{ \"//\", \"/\" },\n+\t{ \"///\", \"/\" },\n+\t{ \"////\", \"/\" },\n+#endif\n+\t{ \"usr\", \"usr\" },\n+\t{ \"/usr\", \"usr\" },\n+\t{ \"/usr/\", \"usr\" },\n+\t{ \"/usr//\", \"usr\" },\n+\t{ \"/usr/lib\", \"lib\" },\n+\t{ \"usr/lib\", \"lib\" },\n+\t{ \"usr/lib///\", \"lib\" },\n+\n+#if defined(__MINGW32__) || defined(_MSC_VER)\n+\n+\t/* --- win32 type paths --- */\n+\t{ \"\\\\usr\", \"usr\" },\n+\t{ \"\\\\usr\\\\\", \"usr\" },\n+\t{ \"\\\\usr\\\\\\\\\", \"usr\" },\n+\t{ \"\\\\usr\\\\lib\", \"lib\" },\n+\t{ \"usr\\\\lib\", \"lib\" },\n+\t{ \"usr\\\\lib\\\\\\\\\\\\\", \"lib\" },\n+\t{ \"C:/usr\", \"usr\" },\n+\t{ \"C:/usr\", \"usr\" },\n+\t{ \"C:/usr/\", \"usr\" },\n+\t{ \"C:/usr//\", \"usr\" },\n+\t{ \"C:/usr/lib\", \"lib\" },\n+\t{ \"C:usr/lib\", \"lib\" },\n+\t{ \"C:usr/lib///\", \"lib\" },\n+\t{ \"C:\", \".\" },\n+\t{ \"C:a\", \"a\" },\n+\t{ \"C:/\", \"/\" },\n+\t{ \"C:///\", \"/\" },\n+#if defined(NO_LIBGEN_H)\n+\t{ \"\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n+#else\n+\n+\t/* win32 platform variations: */\n+#if defined(__MINGW32__)\n+\t{ \"\\\\\", \"/\" },\n+\t{ \"\\\\\\\\\", \"/\" },\n+\t{ \"\\\\\\\\\\\\\", \"/\" },\n+#endif\n+\n+#if defined(_MSC_VER)\n+\t{ \"\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n+#endif\n+\n+#endif\n+#endif\n+\t{ NULL, NULL }\n+};\n+\n+static struct test_data dirname_data[] = {\n+\t/* --- POSIX type paths --- */\n+\t{ NULL, \".\" },\n+\t{ \"\", \".\" },\n+\t{ \".\", \".\" },\n+\t{ \"..\", \".\" },\n+\t{ \"/\", \"/\" },\n+\t{ \"//\", \"//\" },\n+#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n+\t{ \"///\", \"//\" },\n+\t{ \"////\", \"//\" },\n+#else\n+\t{ \"///\", \"/\" },\n+\t{ \"////\", \"/\" },\n+#endif\n+\t{ \"usr\", \".\" },\n+\t{ \"/usr\", \"/\" },\n+\t{ \"/usr/\", \"/\" },\n+\t{ \"/usr//\", \"/\" },\n+\t{ \"/usr/lib\", \"/usr\" },\n+\t{ \"usr/lib\", \"usr\" },\n+\t{ \"usr/lib///\", \"usr\" },\n+\n+#if defined(__MINGW32__) || defined(_MSC_VER)\n+\n+\t/* --- win32 type paths --- */\n+\t{ \"\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\", \"\\\\\\\\\" },\n+\t{ \"\\\\usr\", \"\\\\\" },\n+\t{ \"\\\\usr\\\\\", \"\\\\\" },\n+\t{ \"\\\\usr\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\usr\\\\lib\", \"\\\\usr\" },\n+\t{ \"usr\\\\lib\", \"usr\" },\n+\t{ \"usr\\\\lib\\\\\\\\\\\\\", \"usr\" },\n+\t{ \"C:a\", \"C:.\" },\n+\t{ \"C:/\", \"C:/\" },\n+\t{ \"C:///\", \"C:/\" },\n+\t{ \"C:/usr\", \"C:/\" },\n+\t{ \"C:/usr/\", \"C:/\" },\n+\t{ \"C:/usr//\", \"C:/\" },\n+\t{ \"C:/usr/lib\", \"C:/usr\" },\n+\t{ \"C:usr/lib\", \"C:usr\" },\n+\t{ \"C:usr/lib///\", \"C:usr\" },\n+\t{ \"\\\\\\\\\\\\\", \"\\\\\" },\n+\t{ \"\\\\\\\\\\\\\\\\\", \"\\\\\" },\n+#if defined(NO_LIBGEN_H)\n+\t{ \"C:\", \"C:.\" },\n+#else\n+\n+\t/* win32 platform variations: */\n+#if defined(__MINGW32__)\n+\t/* the following is clearly wrong ... */\n+\t{ \"C:\", \".\" },\n+#endif\n+\n+#if defined(_MSC_VER)\n+\t{ \"C:\", \"C:.\" },\n+#endif\n+\n+#endif\n+#endif\n+\t{ NULL, NULL }\n+};\n+\n int main(int argc, char **argv)\n {\n \tif (argc == 3 && !strcmp(argv[1], \"normalize_path_copy\")) {\n@@ -133,6 +293,12 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n \n+\tif (argc == 2 && !strcmp(argv[1], \"basename\"))\n+\t\treturn test_function(basename_data, basename, argv[1]);\n+\n+\tif (argc == 2 && !strcmp(argv[1], \"dirname\"))\n+\t\treturn test_function(dirname_data, dirname, argv[1]);\n+\n \tfprintf(stderr, \"%s: unknown function name: %s\\n\", argv[0],\n \t\targv[1] ? argv[1] : \"(there was none)\");\n \treturn 1;\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275853","messageId":"56959DFA.9000704@ramsayjones.plus.com","threadId":"40458","inReplyTo":"cover.1452585382.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v4 0/4] Ensure that we can build without libgen.h","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-01-13T00:44:42Z","receivedAt":"2016-01-13T00:44:42Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nHi Johannes,\n\nSorry for not commenting sooner, I've been away from email for\na few days. Also, I have only just looked at what is currently\nin pu (@1a05310), which I'm pretty sure is v3 of this series.\n\nOn 12/01/16 07:57, Johannes Schindelin wrote:\n> This mini series adds a fall-back for the `dirname()` function that we use\n> e.g. in git-am. This is necessary because not all platforms have a working\n> libgen.h.\n> \n> While at it, we ensure that our basename() drop-in conforms to the POSIX\n> specifications.\n\nI was somewhat disappointed that you ignored the implementation of\ngitbasename() and gitdirname() that was included in the test-libgen.c\nfile that I sent you. I had hoped they would be (at worst) a good starting\npoint if you found them to be lacking for your use case (ie. for the\n64-bit versions of MSVC/MinGW).\n\nDid you have any test cases that failed? (If so, could you please add\nthem to the tests).\n\nHmm, I just had another look at them and recalled one of my TODO items.\nAhem, yes, ... err, replace code which provoked undefined behaviour. :-P\n\nActually, that took just ten minutes to fix. (patch below)\n\n> \n> In addition to Eric's style improvement, v4 also fixes the signature\n> of skip_dos_drive_prefix() in the non-Windows case.\n\nYes, this fixes one of my comments about v3.\n\nATB,\nRamsay Jones\n-- >8 --\nFrom: Ramsay Jones \nDate: Tue, 12 Jan 2016 23:28:09 +0000\nSubject: [PATCH] test-libgen.c: don't provoke undefined behaviour\n\n---\n test-libgen.c | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/test-libgen.c b/test-libgen.c\nindex aa3bd18..3024bf1 100644\n--- a/test-libgen.c\n+++ b/test-libgen.c\n@@ -42,9 +42,11 @@ char *gitbasename (char *path)\n \t\t*p-- = '\\0';\n \t}\n \t/* find begining of last path component */\n-\twhile (p >= path && !is_dir_sep(*p))\n+\twhile (p > path && !is_dir_sep(*p))\n \t\tp--;\n-\treturn p + 1;\n+\tif (is_dir_sep(*p))\n+\t\tp++;\n+\treturn p;\n }\n \n char *gitdirname(char *path)\n@@ -71,13 +73,12 @@ char *gitdirname(char *path)\n \t\t*p-- = '\\0';\n \t}\n \t/* find begining of last path component */\n-\twhile (p >= start && !is_dir_sep(*p))\n+\twhile (p > start && !is_dir_sep(*p))\n \t\tp--;\n \t/* terminate dirname */\n-\tif (p < start) {\n-\t\tp = start;\n+\tif (p == start && !is_dir_sep(*p))\n \t\t*p++ = '.';\n-\t} else if (p == start)\n+\telse if (p == start)\n \t\tp++;\n \t*p = '\\0';\n \treturn path;\n-- \n2.7.0\n"},{"id":"275854","messageId":"56959EFF.6050207@ramsayjones.plus.com","threadId":"40458","inReplyTo":"a7375faaba405354b30bc19c6edbdb1ef7c68ab1.1452585382.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v4 2/4] compat/basename: make basename() conform to POSIX","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-01-13T00:49:03Z","receivedAt":"2016-01-13T00:49:03Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 12/01/16 07:57, Johannes Schindelin wrote:\n> According to POSIX, basename(\"/path/\") should return \"path\", not\n> \"path/\". Likewise, basename(NULL) and basename(\"\") should both\n> return \".\" to conform.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> compat/basename.c | 20 +++++++++++++++++---\n> 1 file changed, 17 insertions(+), 3 deletions(-)\n> \n> diff --git a/compat/basename.c b/compat/basename.c\n> index 9f00421..0f1b0b0 100644\n> --- a/compat/basename.c\n> +++ b/compat/basename.c\n> @@ -4,10 +4,24 @@\n> char *gitbasename (char *path)\n> {\n> \tconst char *base;\n> -\tskip_dos_drive_prefix(&path);\n> +\n> +\tif (path)\n> +\t\tskip_dos_drive_prefix(&path);\n> +\n> +\tif (!path || !*path)\n> +\t\treturn \".\";\n> +\n> \tfor (base = path; *path; path++) {\n> -\t\tif (is_dir_sep(*path))\n> -\t\t\tbase = path + 1;\n> +\t\tif (!is_dir_sep(*path))\n> +\t\t\tcontinue;\n> +\t\tdo {\n> +\t\t\tpath++;\n> +\t\t} while (is_dir_sep(*path));\n> +\t\tif (*path)\n> +\t\t\tbase = path;\n> +\t\telse\n> +\t\t\twhile (--path != base && is_dir_sep(*path))\n> +\t\t\t\t*path = '\\0';\n> \t}\n> \treturn (char *)base;\n> }\n> \n\nI don't suppose it makes much difference, but I find my version\nslightly easier to read:\n\nchar *gitbasename (char *path)\n{\n\tchar *p;\n\n\tif (!path || !*path)\n\t\treturn \".\";\n\t/* skip drive designator, if any */\n\tif (has_dos_drive_prefix(path))\n\t\tpath += 2;\n\tif (!*path)\n\t\treturn \".\";\n\t/* trim trailing directory separators */\n\tp = path + strlen(path) - 1;\n\twhile (is_dir_sep(*p)) {\n\t\tif (p == path)\n\t\t\treturn path;\n\t\t*p-- = '\\0';\n\t}\n\t/* find begining of last path component */\n\twhile (p > path && !is_dir_sep(*p))\n\t\tp--;\n\tif (is_dir_sep(*p))\n\t\tp++;\n\treturn p;\n}\n\nATB,\nRamsay Jones\n"},{"id":"275855","messageId":"5695A077.7070606@ramsayjones.plus.com","threadId":"40458","inReplyTo":"04a7a497f9a5771d4dbf5fd605f138607b2bae0a.1452585382.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v4 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-01-13T00:55:19Z","receivedAt":"2016-01-13T00:55:19Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 12/01/16 07:57, Johannes Schindelin wrote:\n> When there is no `libgen.h` to our disposal, we miss the `dirname()`\n> function.\n> \n> So far, we only had one user of that function: credential-cache--daemon\n> (which was only compiled when Unix sockets are available, anyway). But\n> now we also have `builtin/am.c` as user, so we need it.\n> \n> Since `dirname()` is a sibling of `basename()`, we simply put our very\n> own `gitdirname()` implementation next to `gitbasename()` and use it\n> if `NO_LIBGEN_H` has been set.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> compat/basename.c | 44 ++++++++++++++++++++++++++++++++++++++++++++\n> git-compat-util.h | 2 ++\n> 2 files changed, 46 insertions(+)\n> \n> diff --git a/compat/basename.c b/compat/basename.c\n> index 0f1b0b0..96bd953 100644\n> --- a/compat/basename.c\n> +++ b/compat/basename.c\n> @@ -1,4 +1,5 @@\n> #include \"../git-compat-util.h\"\n> +#include \"../strbuf.h\"\n> \n> /* Adapted from libiberty's basename.c. */\n> char *gitbasename (char *path)\n> @@ -25,3 +26,46 @@ char *gitbasename (char *path)\n> \t}\n> \treturn (char *)base;\n> }\n> +\n> +char *gitdirname(char *path)\n> +{\n> +\tstatic struct strbuf buf = STRBUF_INIT;\n> +\tchar *p = path, *slash = NULL, c;\n> +\tint dos_drive_prefix;\n> +\n> +\tif (!p)\n> +\t\treturn \".\";\n> +\n> +\tif ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p)\n> +\t\tgoto dot;\n> +\n> +\t/*\n> +\t * POSIX.1-2001 says dirname(\"/\") should return \"/\", and dirname(\"//\")\n> +\t * should return \"//\", but dirname(\"///\") should return \"/\" again.\n> +\t */\n> +\tif (is_dir_sep(*p)) {\n> +\t\tif (!p[1] || (is_dir_sep(p[1]) && !p[2]))\n> +\t\t\treturn path;\n> +\t\tslash = ++p;\n> +\t}\n> +\twhile ((c = *(p++)))\n> +\t\tif (is_dir_sep(c)) {\n> +\t\t\tchar *tentative = p - 1;\n> +\n> +\t\t\t/* POSIX.1-2001 says to ignore trailing slashes */\n> +\t\t\twhile (is_dir_sep(*p))\n> +\t\t\t\tp++;\n> +\t\t\tif (*p)\n> +\t\t\t\tslash = tentative;\n> +\t\t}\n> +\n> +\tif (slash) {\n> +\t\t*slash = '\\0';\n> +\t\treturn path;\n> +\t}\n> +\n> +dot:\n> +\tstrbuf_reset(&buf);\n> +\tstrbuf_addf(&buf, \"%.*s.\", dos_drive_prefix, path);\n> +\treturn buf.buf;\n> +}\n\nAgain, I find my version much easier to read:\n\nchar *gitdirname(char *path)\n{\n\tchar *p, *start;\n\n\tif (!path || !*path)\n\t\treturn \".\";\n\tstart = path;\n\t/* skip drive designator, if any */\n\tif (has_dos_drive_prefix(path))\n\t\tstart += 2;\n\t/* check for // */\n\tif (strcmp(start, \"//\") == 0)\n\t\treturn path;\n\t/* check for \\\\ */\n\tif (is_dir_sep('\\\\') && strcmp(start, \"\\\\\\\\\") == 0)\n\t\treturn path;\n\t/* trim trailing directory separators */\n\tp = path + strlen(path) - 1;\n\twhile (is_dir_sep(*p)) {\n\t\tif (p == start)\n\t\t\treturn path;\n\t\t*p-- = '\\0';\n\t}\n\t/* find begining of last path component */\n\twhile (p > start && !is_dir_sep(*p))\n\t\tp--;\n\t/* terminate dirname */\n\tif (p == start && !is_dir_sep(*p))\n\t\t*p++ = '.';\n\telse if (p == start)\n\t\tp++;\n\t*p = '\\0';\n\treturn path;\n}\n\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index fbb11bb..5f72f1c 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -253,6 +253,8 @@ struct itimerval {\n> #else\n\nAlso, when compiling on Cygwin with NO_LIBGEN_H, I need to\ninclude the following here:\n\n#undef basename\n\nin order to suppress approx 230 warnings about the redefinition\nof the basename macro.\n\n(I suppose that should go in the previous commit. dunno)\n\n> #define basename gitbasename\n> extern char *gitbasename(char *);\n> +#define dirname gitdirname\n> +extern char *gitdirname(char *);\n> #endif\n> \n> #ifndef NO_ICONV\n> \n\nATB,\nRamsay Jones\n"},{"id":"275856","messageId":"5695A132.1040100@ramsayjones.plus.com","threadId":"40458","inReplyTo":"7d73267984ab029df022477e341c536e111eafdd.1452585382.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v4 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-01-13T00:58:26Z","receivedAt":"2016-01-13T00:58:26Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 12/01/16 07:57, Johannes Schindelin wrote:\n> Unfortunately, some libgen implementations yield outcomes different\n> from what Git expects. For example, mingw-w64-crt provides a basename()\n> function, that shortens `path0/` to `path`!\n> \n> So let's verify that the basename() and dirname() functions we use\n> conform to what Git expects.\n> \n> Derived-from-code-by: Ramsay Jones \n> Signed-off-by: Johannes Schindelin \n> ---\n> t/t0060-path-utils.sh | 3 +\n> test-path-utils.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 169 insertions(+)\n> \n> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\n> index 627ef85..f0152a7 100755\n> --- a/t/t0060-path-utils.sh\n> +++ b/t/t0060-path-utils.sh\n> @@ -59,6 +59,9 @@ case $(uname -s) in\n> \t;;\n> esac\n> \n> +test_expect_success basename 'test-path-utils basename'\n> +test_expect_success dirname 'test-path-utils dirname'\n> +\n> norm_path \"\" \"\"\n> norm_path . \"\"\n> norm_path ./ \"\"\n> diff --git a/test-path-utils.c b/test-path-utils.c\n> index c67bf65..4ab68ac 100644\n> --- a/test-path-utils.c\n> +++ b/test-path-utils.c\n> @@ -39,6 +39,166 @@ static void normalize_argv_string(const char **var, const char *input)\n> \t\tdie(\"Bad value: %s\\n\", input);\n> }\n> \n> +struct test_data {\n> +\tconst char *from; /* input: transform from this ... */\n> +\tconst char *to; /* output: ... to this. */\n> +};\n> +\n> +static int test_function(struct test_data *data, char *(*func)(char *input),\n> +\tconst char *funcname)\n> +{\n> +\tint failed = 0, i;\n> +\tchar buffer[1024];\n> +\tchar *to;\n> +\n> +\tfor (i = 0; data[i].to; i++) {\n> +\t\tif (!data[i].from)\n> +\t\t\tto = func(NULL);\n> +\t\telse {\n> +\t\t\tstrcpy(buffer, data[i].from);\n> +\t\t\tto = func(buffer);\n> +\t\t}\n> +\t\tif (strcmp(to, data[i].to)) {\n> +\t\t\terror(\"FAIL: %s(%s) => '%s' != '%s'\\n\",\n> +\t\t\t\tfuncname, data[i].from, to, data[i].to);\n> +\t\t\tfailed = 1;\n> +\t\t}\n> +\t}\n> +\treturn failed;\n> +}\n> +\n> +static struct test_data basename_data[] = {\n> +\t/* --- POSIX type paths --- */\n> +\t{ NULL, \".\" },\n> +\t{ \"\", \".\" },\n> +\t{ \".\", \".\" },\n> +\t{ \"..\", \"..\" },\n> +\t{ \"/\", \"/\" },\n> +#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n> +\t{ \"//\", \"//\" },\n> +\t{ \"///\", \"//\" },\n> +\t{ \"////\", \"//\" },\n> +#else\n> +\t{ \"//\", \"/\" },\n> +\t{ \"///\", \"/\" },\n> +\t{ \"////\", \"/\" },\n> +#endif\n> +\t{ \"usr\", \"usr\" },\n> +\t{ \"/usr\", \"usr\" },\n> +\t{ \"/usr/\", \"usr\" },\n> +\t{ \"/usr//\", \"usr\" },\n> +\t{ \"/usr/lib\", \"lib\" },\n> +\t{ \"usr/lib\", \"lib\" },\n> +\t{ \"usr/lib///\", \"lib\" },\n> +\n> +#if defined(__MINGW32__) || defined(_MSC_VER)\n> +\n> +\t/* --- win32 type paths --- */\n> +\t{ \"\\\\usr\", \"usr\" },\n> +\t{ \"\\\\usr\\\\\", \"usr\" },\n> +\t{ \"\\\\usr\\\\\\\\\", \"usr\" },\n> +\t{ \"\\\\usr\\\\lib\", \"lib\" },\n> +\t{ \"usr\\\\lib\", \"lib\" },\n> +\t{ \"usr\\\\lib\\\\\\\\\\\\\", \"lib\" },\n> +\t{ \"C:/usr\", \"usr\" },\n> +\t{ \"C:/usr\", \"usr\" },\n\nThis duplication was in the test-libgen.c file I sent\nyou ... so, my bad. ;-)\n\nDid you not have more tests to add?\n\nATB,\nRamsay Jones\n"},{"id":"275860","messageId":"xmqq7fjeqjae.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"56959DFA.9000704@ramsayjones.plus.com","subject":"Re: [PATCH v4 0/4] Ensure that we can build without libgen.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-13T02:56:25Z","receivedAt":"2016-01-13T02:56:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> Hi Johannes,\n> ...\n> I was somewhat disappointed that you ignored the implementation of\n> gitbasename() and gitdirname() that was included in the test-libgen.c\n> file that I sent you. I had hoped they would be (at worst) a good starting\n> point if you found them to be lacking for your use case (ie. for the\n> 64-bit versions of MSVC/MinGW).\n\nSorry to hear that, but the 'next' branch has just been rewound and\nrebuilt with this series, so it is too late to replace them with\nanother round of reroll. It however is never too late to improve\nwith incremental updates, though, so please work together and send\nin a follow-up patch series as/if needed.\n\nThanks.\n"},{"id":"275869","messageId":"5695EB6A.1030402@web.de","threadId":"40458","inReplyTo":"xmqq7fjeqjae.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 0/4] Ensure that we can build without libgen.h","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-01-13T06:15:06Z","receivedAt":"2016-01-13T06:15:06Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 01/13/2016 03:56 AM, Junio C Hamano wrote:\n> Ramsay Jones writes:\n>\n>> Hi Johannes,\n>> ...\n>> I was somewhat disappointed that you ignored the implementation of\n>> gitbasename() and gitdirname() that was included in the test-libgen.c\n>> file that I sent you. I had hoped they would be (at worst) a good starting\n>> point if you found them to be lacking for your use case (ie. for the\n>> 64-bit versions of MSVC/MinGW).\n> Sorry to hear that, but the 'next' branch has just been rewound and\n> rebuilt with this series, so it is too late to replace them with\n> another round of reroll. It however is never too late to improve\n> with incremental updates, though, so please work together and send\n> in a follow-up patch series as/if needed.\n>\nIs there a chance to keep it in next (and not merge to master) until\nwe have found a solution for the broken t0060 test under Mac OS X ?\n"},{"id":"275879","messageId":"alpine.DEB.2.20.1601130759300.2964@virtualbox","threadId":"40458","inReplyTo":"56959DFA.9000704@ramsayjones.plus.com","subject":"Re: [PATCH v4 0/4] Ensure that we can build without libgen.h","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-13T07:02:51Z","receivedAt":"2016-01-13T07:02:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ramsay,\n\nOn Wed, 13 Jan 2016, Ramsay Jones wrote:\n\n> On 12/01/16 07:57, Johannes Schindelin wrote:\n> > This mini series adds a fall-back for the `dirname()` function that we use\n> > e.g. in git-am. This is necessary because not all platforms have a working\n> > libgen.h.\n> > \n> > While at it, we ensure that our basename() drop-in conforms to the POSIX\n> > specifications.\n> \n> I was somewhat disappointed that you ignored the implementation of\n> gitbasename() and gitdirname() that was included in the test-libgen.c\n> file that I sent you.\n\nI am sorry you feel that I ignored your work!\n\nMy line of reasoning, however, was to go with the existing gitbasename()\nand with the gitdirname() I had come up with, because I was already\nfamiliar with them.\n\nYour tests included a couple of corner cases that neither handled\ncorrectly, and I was able to fix that, so I was happy.\n\nTo be quite honest, I blindly deleted everything but the tests, noticed\nthat the remaining code looked eerily similar to test-path-utils, and\nmerged it there.\n\nCiao,\nDscho\n"},{"id":"275880","messageId":"alpine.DEB.2.20.1601130803440.2964@virtualbox","threadId":"40458","inReplyTo":"56959EFF.6050207@ramsayjones.plus.com","subject":"Re: [PATCH v4 2/4] compat/basename: make basename() conform to POSIX","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-13T07:14:09Z","receivedAt":"2016-01-13T07:14:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ramsay,\n\nOn Wed, 13 Jan 2016, Ramsay Jones wrote:\n\n> On 12/01/16 07:57, Johannes Schindelin wrote:\n> > diff --git a/compat/basename.c b/compat/basename.c\n> > index 9f00421..0f1b0b0 100644\n> > --- a/compat/basename.c\n> > +++ b/compat/basename.c\n> > @@ -4,10 +4,24 @@\n> > char *gitbasename (char *path)\n> > {\n> > \tconst char *base;\n> > -\tskip_dos_drive_prefix(&path);\n> > +\n> > +\tif (path)\n> > +\t\tskip_dos_drive_prefix(&path);\n> > +\n> > +\tif (!path || !*path)\n> > +\t\treturn \".\";\n> > +\n> > \tfor (base = path; *path; path++) {\n> > -\t\tif (is_dir_sep(*path))\n> > -\t\t\tbase = path + 1;\n> > +\t\tif (!is_dir_sep(*path))\n> > +\t\t\tcontinue;\n> > +\t\tdo {\n> > +\t\t\tpath++;\n> > +\t\t} while (is_dir_sep(*path));\n> > +\t\tif (*path)\n> > +\t\t\tbase = path;\n> > +\t\telse\n> > +\t\t\twhile (--path != base && is_dir_sep(*path))\n> > +\t\t\t\t*path = '\\0';\n> > \t}\n> > \treturn (char *)base;\n> > }\n> > \n> \n> I don't suppose it makes much difference, but I find my version\n> slightly easier to read:\n\nYours is better documented, yes, but as I said, I started from what Git\nalready had and tried to provide as minimal changes as possible, to make\nreviewing easy. In any case, I am very reluctant when it comes to\nwholesale code replacements: in my experience, these frequently lead to\nnew, entertaining and unintended behavior. I worked with somebody who (for\nthe sake of charity) in the following I will reference only by his most\nfrequent commit message: Dr \"Completely new version\" (and yes, this was\nthe extent of the commit message). If you buy me a beer or three, I will\ngladly tell you all the fun I had trying to find the regressions in that\ncode.\n\nIn short: please accept that my decision to build on the existing code\nrather than replacing it had nothing to do with your code.\n\nCiao,\nDscho\n"},{"id":"275881","messageId":"alpine.DEB.2.20.1601130816520.2964@virtualbox","threadId":"40458","inReplyTo":"5695A132.1040100@ramsayjones.plus.com","subject":"Re: [PATCH v4 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-13T07:17:26Z","receivedAt":"2016-01-13T07:17:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ramsay,\n\nOn Wed, 13 Jan 2016, Ramsay Jones wrote:\n\n> Did you not have more tests to add?\n\nNo, all my testing was manual so far, and well covered by your test cases.\n\nThanks,\nDscho\n"},{"id":"275882","messageId":"alpine.DEB.2.20.1601130836050.2964@virtualbox","threadId":"40458","inReplyTo":"5695EB6A.1030402@web.de","subject":"Re: [PATCH v4 0/4] Ensure that we can build without libgen.h","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-13T07:38:28Z","receivedAt":"2016-01-13T07:38:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Torsten,\n\nOn Wed, 13 Jan 2016, Torsten Bögershausen wrote:\n\n> On 01/13/2016 03:56 AM, Junio C Hamano wrote:\n> > Ramsay Jones writes:\n> >\n> > > Hi Johannes,\n> > > ...\n> > > I was somewhat disappointed that you ignored the implementation of\n> > > gitbasename() and gitdirname() that was included in the test-libgen.c\n> > > file that I sent you. I had hoped they would be (at worst) a good\n> > > starting\n> > > point if you found them to be lacking for your use case (ie. for the\n> > > 64-bit versions of MSVC/MinGW).\n> > Sorry to hear that, but the 'next' branch has just been rewound and\n> > rebuilt with this series, so it is too late to replace them with\n> > another round of reroll. It however is never too late to improve\n> > with incremental updates, though, so please work together and send\n> > in a follow-up patch series as/if needed.\n> >\n> Is there a chance to keep it in next (and not merge to master) until\n> we have found a solution for the broken t0060 test under Mac OS X ?\n\nThis is actually independent of follow-up patches that might replace the\ngitbasename()/gitdirname() functions wholesale, as the problem you\nencountered is *differing* behavior: no matter what implementation of\ngitdirname() we use, it will either agree with dirname() on Linux or with\ndirname() on MacOSX, it cannot do both.\n\nSo here is a hot-fix (Junio, would you kindly apply it?):\n\n-- snipsnap --\nSubject: [PATCH] t0060: fix dirname test on MacOSX\n\nContrary to this developer's assumptions, the behavior of dirname(\"//\") is\nnot consistent between platforms. Let's document this by not removing this\ntest case, but showing that it behaves differently on MacOSX (unless\nNO_LIBGEN_H is defined, that is).\n\nPointed-out-by: Torsten Bögershausen \nSigned-off-by: Johannes Schindelin \n---\n test-path-utils.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex 4ab68ac..a3a69ce 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -142,7 +142,11 @@ static struct test_data dirname_data[] = {\n \t{ \".\", \".\" },\n \t{ \"..\", \".\" },\n \t{ \"/\", \"/\" },\n+#if defined(__APPLE__) && !defined(NO_LIBGEN_H)\n+\t{ \"//\", \"/\" },\n+#else\n \t{ \"//\", \"//\" },\n+#endif\n #if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n \t{ \"///\", \"//\" },\n \t{ \"////\", \"//\" },\n-- \n2.6.3.windows.1.300.g1c25e49\n"},{"id":"275883","messageId":"alpine.DEB.2.20.1601130838430.2964@virtualbox","threadId":"40458","inReplyTo":"5695A077.7070606@ramsayjones.plus.com","subject":"Re: [PATCH v4 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-13T07:40:12Z","receivedAt":"2016-01-13T07:40:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ramsay,\n\nOn Wed, 13 Jan 2016, Ramsay Jones wrote:\n\n> Also, when compiling on Cygwin with NO_LIBGEN_H, I need to\n> include the following here:\n> \n> #undef basename\n> \n> in order to suppress approx 230 warnings about the redefinition\n> of the basename macro.\n> \n> (I suppose that should go in the previous commit. dunno)\n\nI think this is an incorrect use of NO_LIBGEN_H (because Cygwin obviously\nhas it), but in any case, it is a completely independent issue from\nfixing/testing basename()/dirname(), so your #undef basename should be in\na completely separate commit, methinks.\n\nCiao,\nDscho\n"},{"id":"275885","messageId":"alpine.DEB.2.20.1601131022410.2964@virtualbox","threadId":"40458","inReplyTo":"5695E4FB.2060705@web.de","subject":"Re: [PATCH v4 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-13T09:27:36Z","receivedAt":"2016-01-13T09:27:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Torsten,\n\nOn Wed, 13 Jan 2016, Torsten Bögershausen wrote:\n\n> On 01/12/2016 08:57 AM, Johannes Schindelin wrote:\n> \n> > +static struct test_data basename_data[] = {\n> > +\t/* --- POSIX type paths --- */\n> > +\t{ NULL, \".\" },\n> > +\t{ \"\", \".\" },\n> > +\t{ \".\", \".\" },\n> > +\t{ \"..\", \"..\" },\n> > +\t{ \"/\", \"/\" },\n> > +#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n> Why the !defined(NO_LIBGEN_H)\n> \n> Shouldn't CYGWIN always behave the same ?\n\nOne would assume... Alas, it does not.\n\nI inherited the code in question and wondered the same. I opted for\nkeeping the code as a documentation of the differing behavior.\n\n> The main problem is, that t0060 fails under Mac OS (with mac ports\n> installed):\n> expecting success: test-path-utils dirname\n> error: FAIL: dirname(//) => '/' != '//'\n\nSee the patch I sent this morning.\n\nCiao,\nDscho"},{"id":"275917","messageId":"56967CA3.7040103@ramsayjones.plus.com","threadId":"40458","inReplyTo":"5695E4FB.2060705@web.de","subject":"Re: [PATCH v4 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-01-13T16:34:43Z","receivedAt":"2016-01-13T16:34:43Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 13/01/16 05:47, Torsten Bögershausen wrote:\n> On 01/12/2016 08:57 AM, Johannes Schindelin wrote:\n> \n[snip]\n\n>> +\n>> +static struct test_data basename_data[] = {\n>> +\t/* --- POSIX type paths --- */\n>> +\t{ NULL, \".\" },\n>> +\t{ \"\", \".\" },\n>> +\t{ \".\", \".\" },\n>> +\t{ \"..\", \"..\" },\n>> +\t{ \"/\", \"/\" },\n>> +#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n> Why the !defined(NO_LIBGEN_H)\n\nThese tests were derived from a standalone test program which\nI was using to test my implementation of gitbasename() *and*\nto document the differences between it and the system versions\nof the basename(). (I didn't bother with the GNU version\nof basename, which is somewhat strange).\n\nThis particular section documents what is almost certainly a bug\nin the cygwin basename() and also documents my choice of 'fix'.\n(ie. in my implementation I chose to return '/' for '//', which\nis one of the possible options that POSIX allows.)\n\n> \n> Shouldn't CYGWIN always behave the same ?\n> And, in general, shouldn't all Windows version behave the same ?\n\nHmm, cygwin is not really a 'Windows version', so ... (We have been\ncaught out before by cygwin 'supporting' UNC paths, so support for\n'//' is open to question. Also, some git programs on cygwin kinda\nsorta support dos paths ...)\n\n> (This would mean, that we always use ../compat/basename.c for all kind\n> of Windows Git implementattion. Would there be a drawback ?)\n\nAnd maybe not just Windows ...\n\n> \n> \n> The main problem is, that t0060 fails under Mac OS (with mac ports installed):\n> expecting success: test-path-utils dirname\n> error: FAIL: dirname(//) => '/' != '//'\n\nYep, not surprised. Again that test file was developed and tested on\nonly the five platforms available to me at the time, namely: Linux (both\n32 and 64bit), Windows XP 32-bit (MSVC), MinGW 32-bit and Cygwin 32-bit.\n\nPOSIX says, in part [1]:\n\n If the string pointed to by path consists entirely of the '/' character,\n basename() shall return a pointer to the string \"/\". If the string pointed\n to by path is exactly \"//\", it is implementation-defined whether '/' or \"//\"\n is returned.\n\n[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/basename.html\n\nSo we should expect other systems to differ, even if they support POSIX. (and maybe\nnot just this test case.)\n\n> \n> not ok 2 - dirname\n> # test-path-utils dirname\n> \n> To my understanding the treatment of a path name like \"//\"\n> is defined as \"undefined\":\n> If /string/ is \"//\", it is implementation-defined whether steps 3 to 6 are skipped or processed.\n> http://pubs.opengroup.org/onlinepubs/009695399/utilities/basename.html\n> \n> What I understand is that a path like \"//XX/YY/ZZ\" can be handled in 3 different ways:\n> a) Same as \"/XX/YY/ZZ\", silently turning \"//\" into \"/\"\n> b) Same as \"\\\\XX\\YY\\ZZ\", using UNC names under Windows. # Note: this reads as \"\\\\\\\\XX\\\\YY\\\\ZZ\" in a C program\n> c) As invalid\n> \n> \n> Does it make sense to use the compat/basename.c for all Git implementations ?\n> (Or at least for CYGWIN, Mac OS, MSYS, MSVC)\n> \n\nMaybe, I hadn't got that far yet ... :-D\n\nATB,\nRamsay Jones\n"},{"id":"275919","messageId":"56967EF5.60202@ramsayjones.plus.com","threadId":"40458","inReplyTo":"alpine.DEB.2.20.1601130838430.2964@virtualbox","subject":"Re: [PATCH v4 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-01-13T16:44:37Z","receivedAt":"2016-01-13T16:44:37Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 13/01/16 07:40, Johannes Schindelin wrote:\n> Hi Ramsay,\n> \n> On Wed, 13 Jan 2016, Ramsay Jones wrote:\n> \n>> Also, when compiling on Cygwin with NO_LIBGEN_H, I need to\n>> include the following here:\n>>\n>> #undef basename\n>>\n>> in order to suppress approx 230 warnings about the redefinition\n>> of the basename macro.\n>>\n>> (I suppose that should go in the previous commit. dunno)\n> \n> I think this is an incorrect use of NO_LIBGEN_H (because Cygwin obviously\n> has it), but in any case, it is a completely independent issue from\n> fixing/testing basename()/dirname(), so your #undef basename should be in\n> a completely separate commit, methinks.\n\nOK. I think this worked fine on 32-bit cygwin, but the system headers\nhave changed quite a bit on 64-bit cygwin and I only tried it for the\nfirst time yesterday. (It was helpful in the debugging process at one\npoint to be able to build with NO_LIBGEN_H on all platforms ...)\n\nATB,\nRamsay Jones\n"},{"id":"275921","messageId":"56967F7B.8080407@ramsayjones.plus.com","threadId":"40458","inReplyTo":"alpine.DEB.2.20.1601131022410.2964@virtualbox","subject":"Re: [PATCH v4 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-01-13T16:46:51Z","receivedAt":"2016-01-13T16:46:51Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 13/01/16 09:27, Johannes Schindelin wrote:\n> Hi Torsten,\n> \n> On Wed, 13 Jan 2016, Torsten Bögershausen wrote:\n> \n>> On 01/12/2016 08:57 AM, Johannes Schindelin wrote:\n>>\n>>> +static struct test_data basename_data[] = {\n>>> +\t/* --- POSIX type paths --- */\n>>> +\t{ NULL, \".\" },\n>>> +\t{ \"\", \".\" },\n>>> +\t{ \".\", \".\" },\n>>> +\t{ \"..\", \"..\" },\n>>> +\t{ \"/\", \"/\" },\n>>> +#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n>> Why the !defined(NO_LIBGEN_H)\n>>\n>> Shouldn't CYGWIN always behave the same ?\n> \n> One would assume... Alas, it does not.\n\nErr, ... yes it does! :-P\n\n> \n> I inherited the code in question and wondered the same. I opted for\n> keeping the code as a documentation of the differing behavior.\n\nExactly.\n\nATB,\nRamsay Jones\n"},{"id":"275935","messageId":"xmqqh9ihpfav.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"56967CA3.7040103@ramsayjones.plus.com","subject":"Re: [PATCH v4 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-13T17:20:08Z","receivedAt":"2016-01-13T17:20:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> This particular section documents what is almost certainly a bug\n> in the cygwin basename() and also documents my choice of 'fix'.\n> (ie. in my implementation I chose to return '/' for '//', which\n> is one of the possible options that POSIX allows.)\n> ...\n> POSIX says, in part [1]:\n>\n> If the string pointed to by path consists entirely of the '/' character,\n> basename() shall return a pointer to the string \"/\". If the string pointed\n> to by path is exactly \"//\", it is implementation-defined whether '/' or \"//\"\n> is returned.\n>\n> [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/basename.html\n>\n> So we should expect other systems to differ, even if they support POSIX. (and maybe\n> not just this test case.)\n\nDoesn't that mean the test shouldn't be insisting on the output\nbeing one that you arbitrarily pick? It feels to me that it is\nwrong to say \"We require // to become / unless we know we are on\nsuch and such systems\". Instead, shouldn't it be doing \"We feed //\nto the function. Either / or // is acceptable; any other value is a\nbug\"?\n\nIt is tempting to have a \"check\" feature to the test program for the\ncurious which one of the two acceptable results a particular platform\n(or more precisely, implementation of basename/dirname) produces, but\nI do not think \"test\" feature should insist on / and reject //.\n"},{"id":"275958","messageId":"CAO2U3QjYukb4mB414mVLX2=CxLPBnDaUyDRsfitE_bTZv8_zFQ@mail.gmail.com","threadId":"40458","inReplyTo":"eca740dbf6271bd69f2ccb14163175996ef7c837.1452270051.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Michael Blume","fromEmail":"blume.mike@gmail.com","sentAt":"2016-01-13T18:52:38Z","receivedAt":"2016-01-13T18:52:38Z","isPatch":true,"sender":{"key":"blume.mike@gmail.com","avatar":"https://gravatar.com/avatar/1a7b440e1d942425ff4098ac7fc15b86b30cecaa56e1692a7ef8b5939ba25ea7?d=mp&s=160"},"body":"On Fri, Jan 8, 2016 at 8:21 AM, Johannes Schindelin\n wrote:\n> Unfortunately, some libgen implementations yield outcomes different from\n> what Git expects. For example, mingw-w64-crt provides a basename()\n> function, that shortens `path0/` to `path`!\n>\n> So let's verify that the basename() and dirname() functions we use conform\n> to what Git expects.\n>\n> Derived-from-code-by: Ramsay Jones \n> Signed-off-by: Johannes Schindelin \n> ---\n> t/t0060-path-utils.sh | 3 +\n> test-path-utils.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 171 insertions(+)\n>\n> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\n> index 627ef85..f0152a7 100755\n> --- a/t/t0060-path-utils.sh\n> +++ b/t/t0060-path-utils.sh\n> @@ -59,6 +59,9 @@ case $(uname -s) in\n> ;;\n> esac\n>\n> +test_expect_success basename 'test-path-utils basename'\n> +test_expect_success dirname 'test-path-utils dirname'\n> +\n> norm_path \"\" \"\"\n> norm_path . \"\"\n> norm_path ./ \"\"\n> diff --git a/test-path-utils.c b/test-path-utils.c\n> index c67bf65..74e74c9 100644\n> --- a/test-path-utils.c\n> +++ b/test-path-utils.c\n> @@ -39,6 +39,168 @@ static void normalize_argv_string(const char **var, const char *input)\n> die(\"Bad value: %s\\n\", input);\n> }\n>\n> +struct test_data {\n> + char *from; /* input: transform from this ... */\n> + char *to; /* output: ... to this. */\n> +};\n> +\n> +static int test_function(struct test_data *data, char *(*func)(char *input),\n> + const char *funcname)\n> +{\n> + int failed = 0, i;\n> + static char buffer[1024];\n> + char *to;\n> +\n> + for (i = 0; data[i].to; i++) {\n> + if (!data[i].from)\n> + to = func(NULL);\n> + else {\n> + strcpy(buffer, data[i].from);\n> + to = func(buffer);\n> + }\n> + if (strcmp(to, data[i].to)) {\n> + error(\"FAIL: %s(%s) => '%s' != '%s'\\n\",\n> + funcname, data[i].from, to, data[i].to);\n> + failed++;\n> + }\n> + }\n> + return !!failed;\n> +}\n> +\n> +static struct test_data basename_data[] = {\n> + /* --- POSIX type paths --- */\n> + { NULL, \".\" },\n> + { \"\", \".\" },\n> + { \".\", \".\" },\n> + { \"..\", \"..\" },\n> + { \"/\", \"/\" },\n> +#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n> + { \"//\", \"//\" },\n> + { \"///\", \"//\" },\n> + { \"////\", \"//\" },\n> +#else\n> + { \"//\", \"/\" },\n> + { \"///\", \"/\" },\n> + { \"////\", \"/\" },\n> +#endif\n> + { \"usr\", \"usr\" },\n> + { \"/usr\", \"usr\" },\n> + { \"/usr/\", \"usr\" },\n> + { \"/usr//\", \"usr\" },\n> + { \"/usr/lib\", \"lib\" },\n> + { \"usr/lib\", \"lib\" },\n> + { \"usr/lib///\", \"lib\" },\n> +\n> +#if defined(__MINGW32__) || defined(_MSC_VER)\n> +\n> + /* --- win32 type paths --- */\n> + { \"\\\\usr\", \"usr\" },\n> + { \"\\\\usr\\\\\", \"usr\" },\n> + { \"\\\\usr\\\\\\\\\", \"usr\" },\n> + { \"\\\\usr\\\\lib\", \"lib\" },\n> + { \"usr\\\\lib\", \"lib\" },\n> + { \"usr\\\\lib\\\\\\\\\\\\\", \"lib\" },\n> + { \"C:/usr\", \"usr\" },\n> + { \"C:/usr\", \"usr\" },\n> + { \"C:/usr/\", \"usr\" },\n> + { \"C:/usr//\", \"usr\" },\n> + { \"C:/usr/lib\", \"lib\" },\n> + { \"C:usr/lib\", \"lib\" },\n> + { \"C:usr/lib///\", \"lib\" },\n> + { \"C:\", \".\" },\n> + { \"C:a\", \"a\" },\n> + { \"C:/\", \"/\" },\n> + { \"C:///\", \"/\" },\n> +#if defined(NO_LIBGEN_H)\n> + { \"\\\\\", \"\\\\\" },\n> + { \"\\\\\\\\\", \"\\\\\" },\n> + { \"\\\\\\\\\\\\\", \"\\\\\" },\n> +#else\n> +\n> + /* win32 platform variations: */\n> +#if defined(__MINGW32__)\n> + { \"\\\\\", \"/\" },\n> + { \"\\\\\\\\\", \"/\" },\n> + { \"\\\\\\\\\\\\\", \"/\" },\n> +#endif\n> +\n> +#if defined(_MSC_VER)\n> + { \"\\\\\", \"\\\\\" },\n> + { \"\\\\\\\\\", \"\\\\\" },\n> + { \"\\\\\\\\\\\\\", \"\\\\\" },\n> +#endif\n> +\n> +#endif\n> +#endif\n> + { NULL, \".\" },\n> + { NULL, NULL }\n> +};\n> +\n> +static struct test_data dirname_data[] = {\n> + /* --- POSIX type paths --- */\n> + { NULL, \".\" },\n> + { \"\", \".\" },\n> + { \".\", \".\" },\n> + { \"..\", \".\" },\n> + { \"/\", \"/\" },\n> + { \"//\", \"//\" },\n> +#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)\n> + { \"///\", \"//\" },\n> + { \"////\", \"//\" },\n> +#else\n> + { \"///\", \"/\" },\n> + { \"////\", \"/\" },\n> +#endif\n> + { \"usr\", \".\" },\n> + { \"/usr\", \"/\" },\n> + { \"/usr/\", \"/\" },\n> + { \"/usr//\", \"/\" },\n> + { \"/usr/lib\", \"/usr\" },\n> + { \"usr/lib\", \"usr\" },\n> + { \"usr/lib///\", \"usr\" },\n> +\n> +#if defined(__MINGW32__) || defined(_MSC_VER)\n> +\n> + /* --- win32 type paths --- */\n> + { \"\\\\\", \"\\\\\" },\n> + { \"\\\\\\\\\", \"\\\\\\\\\" },\n> + { \"\\\\usr\", \"\\\\\" },\n> + { \"\\\\usr\\\\\", \"\\\\\" },\n> + { \"\\\\usr\\\\\\\\\", \"\\\\\" },\n> + { \"\\\\usr\\\\lib\", \"\\\\usr\" },\n> + { \"usr\\\\lib\", \"usr\" },\n> + { \"usr\\\\lib\\\\\\\\\\\\\", \"usr\" },\n> + { \"C:a\", \"C:.\" },\n> + { \"C:/\", \"C:/\" },\n> + { \"C:///\", \"C:/\" },\n> + { \"C:/usr\", \"C:/\" },\n> + { \"C:/usr/\", \"C:/\" },\n> + { \"C:/usr//\", \"C:/\" },\n> + { \"C:/usr/lib\", \"C:/usr\" },\n> + { \"C:usr/lib\", \"C:usr\" },\n> + { \"C:usr/lib///\", \"C:usr\" },\n> + { \"\\\\\\\\\\\\\", \"\\\\\" },\n> + { \"\\\\\\\\\\\\\\\\\", \"\\\\\" },\n> +#if defined(NO_LIBGEN_H)\n> + { \"C:\", \"C:.\" },\n> +#else\n> +\n> + /* win32 platform variations: */\n> +#if defined(__MINGW32__)\n> + /* the following is clearly wrong ... */\n> + { \"C:\", \".\" },\n> +#endif\n> +\n> +#if defined(_MSC_VER)\n> + { \"C:\", \"C:.\" },\n> +#endif\n> +\n> +#endif\n> +#endif\n> + { NULL, \".\" },\n> + { NULL, NULL }\n> +};\n> +\n> int main(int argc, char **argv)\n> {\n> if (argc == 3 && !strcmp(argv[1], \"normalize_path_copy\")) {\n> @@ -133,6 +295,12 @@ int main(int argc, char **argv)\n> return 0;\n> }\n>\n> + if (argc == 2 && !strcmp(argv[1], \"basename\"))\n> + return test_function(basename_data, basename, argv[1]);\n> +\n> + if (argc == 2 && !strcmp(argv[1], \"dirname\"))\n> + return test_function(dirname_data, dirname, argv[1]);\n> +\n> fprintf(stderr, \"%s: unknown function name: %s\\n\", argv[0],\n> argv[1] ? argv[1] : \"(there was none)\");\n> return 1;\n> --\n> 2.6.3.windows.1.300.g1c25e49\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\nTest fails on my Mac:\n\nexpecting success: test-path-utils dirname\nerror: FAIL: dirname(//) => '/' != '//'\n\nnot ok 2 - dirname\n# test-path-utils dirname\n"},{"id":"275957","messageId":"alpine.DEB.2.20.1601131953010.2964@virtualbox","threadId":"40458","inReplyTo":"xmqqh9ihpfav.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-13T18:53:58Z","receivedAt":"2016-01-13T18:53:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 13 Jan 2016, Junio C Hamano wrote:\n\n> Ramsay Jones writes:\n> \n> > This particular section documents what is almost certainly a bug\n> > in the cygwin basename() and also documents my choice of 'fix'. (ie.\n> > in my implementation I chose to return '/' for '//', which is one of\n> > the possible options that POSIX allows.)\n> > ...\n> > POSIX says, in part [1]:\n> >\n> > If the string pointed to by path consists entirely of the '/'\n> > character, basename() shall return a pointer to the string \"/\". If\n> > the string pointed to by path is exactly \"//\", it is\n> > implementation-defined whether '/' or \"//\" is returned.\n> >\n> > [1]\n> > http://pubs.opengroup.org/onlinepubs/9699919799/functions/basename.html\n> >\n> > So we should expect other systems to differ, even if they support POSIX. (and maybe\n> > not just this test case.)\n> \n> Doesn't that mean the test shouldn't be insisting on the output\n> being one that you arbitrarily pick? It feels to me that it is\n> wrong to say \"We require // to become / unless we know we are on\n> such and such systems\". Instead, shouldn't it be doing \"We feed //\n> to the function. Either / or // is acceptable; any other value is a\n> bug\"?\n\nI guess that is the best solution of all. I'll try to modify\ntest-path-utils.c accordingly tomorrow.\n\nCiao,\nDscho\n"},{"id":"275967","messageId":"xmqqziw9mfil.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"CAO2U3QjYukb4mB414mVLX2=CxLPBnDaUyDRsfitE_bTZv8_zFQ@mail.gmail.com","subject":"Re: [PATCH v2 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-13T19:43:46Z","receivedAt":"2016-01-13T19:43:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Blume writes:\n\n> Test fails on my Mac:\n>\n> expecting success: test-path-utils dirname\n> error: FAIL: dirname(//) => '/' != '//'\n>\n> not ok 2 - dirname\n> # test-path-utils dirname\n\nThanks for reporting.\n\nRamsay gave a nice analysis at\n\n http://thread.gmane.org/gmane.comp.version-control.git/283928\n\nand Dscho already is working on it, IIUC.\n\nThanks.\n"},{"id":"276006","messageId":"alpine.DEB.2.20.1601140702260.2964@virtualbox","threadId":"40458","inReplyTo":"xmqqziw9mfil.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 4/4] t0060: verify that basename() and dirname() work as expected","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-14T06:09:01Z","receivedAt":"2016-01-14T06:09:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\n[thanks, Junio, for culling the quoted text to the essential part]\n\nOn Wed, 13 Jan 2016, Junio C Hamano wrote:\n\n> Michael Blume writes:\n> \n> > Test fails on my Mac:\n> >\n> > expecting success: test-path-utils dirname\n> > error: FAIL: dirname(//) => '/' != '//'\n> >\n> > not ok 2 - dirname\n> > # test-path-utils dirname\n> \n> Thanks for reporting.\n> \n> Ramsay gave a nice analysis at\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/283928\n> \n> and Dscho already is working on it, IIUC.\n\nI already provided a hot fix:\nhttp://article.gmane.org/gmane.comp.version-control.git/283893\n\nAnd yes, I also work on a proper fix.\n\nCiao,\nDscho\n"},{"id":"276563","messageId":"56A279DA.8080809@kdbg.org","threadId":"40458","inReplyTo":"05cb9e00756e8a364f972cd227804764f6a6380c.1452585382.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v4 1/4] Refactor skipping DOS drive prefixes","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-01-22T18:50:02Z","receivedAt":"2016-01-22T18:50:02Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.01.2016 um 08:57 schrieb Johannes Schindelin:\n> diff --git a/compat/mingw.h b/compat/mingw.h\n> index 57ca477..b3e5044 100644\n> --- a/compat/mingw.h\n> +++ b/compat/mingw.h\n> @@ -361,7 +361,15 @@ HANDLE winansi_get_osfhandle(int fd);\n> * git specific compatibility\n> */\n> \n> -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')\n> +#define has_dos_drive_prefix(path) \\\n> +\t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n> +static inline int mingw_skip_dos_drive_prefix(char **path)\n> +{\n> +\tint ret = has_dos_drive_prefix(*path);\n> +\t*path += ret;\n> +\treturn ret;\n> +}\n> +#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n\nThis triggers\n\n CC alloc.o\nIn file included from git-compat-util.h:186,\n from cache.h:4,\n from alloc.c:12:\ncompat/mingw.h: In function 'mingw_skip_dos_drive_prefix':\ncompat/mingw.h:365: warning: implicit declaration of function 'isalpha'\n\nwhen I build under the old MSYS environment. While I would understand\nthat the old MSYS environment is end-of-lifed and not worth your time\ncatering to, the error is still an indication of a problem.\n\nNotice that mingw.h is #included in line 186 of git-compat-util.h,\nisalpha is only (re-)defined much later in line 790. That would explain\nthe warning. What I do not understand is that you do not observe the\nsame warning in your MSYS2/MINGWxx environment. It would mean that\n is included somewhere.\n\nAt any rate, the resulting binary sometimes uses an isalpha\nimplementation other than the one provided in git-compat-util.h. The\nresult is most likely correct, but it is certainly not the intent,\nis it?\n\nI did not attempt to build with MSVC, but it is not unlikely that it\nshows the same error.\n\nI suggest to move the function definition out of line:\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 10a51c0..0cebb61 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1915,6 +1915,13 @@ pid_t waitpid(pid_t pid, int *status, int options)\n \treturn -1;\n }\n \n+int mingw_skip_dos_drive_prefix(char **path)\n+{\n+\tint ret = has_dos_drive_prefix(*path);\n+\t*path += ret;\n+\treturn ret;\n+}\n+\n int mingw_offset_1st_component(const char *path)\n {\n \tchar *pos = (char *)path;\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 9b5db4e..2099b79 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -360,12 +360,7 @@ HANDLE winansi_get_osfhandle(int fd);\n \n #define has_dos_drive_prefix(path) \\\n \t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n-static inline int mingw_skip_dos_drive_prefix(char **path)\n-{\n-\tint ret = has_dos_drive_prefix(*path);\n-\t*path += ret;\n-\treturn ret;\n-}\n+int mingw_skip_dos_drive_prefix(char **path);\n #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n static inline char *mingw_find_last_dir_sep(const char *path)\n"},{"id":"276569","messageId":"xmqq60ylv3bk.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"56A279DA.8080809@kdbg.org","subject":"Re: [PATCH v4 1/4] Refactor skipping DOS drive prefixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-22T19:09:35Z","receivedAt":"2016-01-22T19:09:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> I suggest to move the function definition out of line:\n>\n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index 10a51c0..0cebb61 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -1915,6 +1915,13 @@ pid_t waitpid(pid_t pid, int *status, int options)\n> \treturn -1;\n> }\n> \n> +int mingw_skip_dos_drive_prefix(char **path)\n> +{\n> +\tint ret = has_dos_drive_prefix(*path);\n> +\t*path += ret;\n> +\treturn ret;\n> +}\n> +\n> int mingw_offset_1st_component(const char *path)\n> {\n> \tchar *pos = (char *)path;\n> diff --git a/compat/mingw.h b/compat/mingw.h\n> index 9b5db4e..2099b79 100644\n> --- a/compat/mingw.h\n> +++ b/compat/mingw.h\n> @@ -360,12 +360,7 @@ HANDLE winansi_get_osfhandle(int fd);\n> \n> #define has_dos_drive_prefix(path) \\\n> \t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n> -static inline int mingw_skip_dos_drive_prefix(char **path)\n> -{\n> -\tint ret = has_dos_drive_prefix(*path);\n> -\t*path += ret;\n> -\treturn ret;\n> -}\n> +int mingw_skip_dos_drive_prefix(char **path);\n> #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n> #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n> static inline char *mingw_find_last_dir_sep(const char *path)\n\nThis sounds good to me. Dscho?\n"},{"id":"276609","messageId":"alpine.DEB.2.20.1601230924090.2964@virtualbox","threadId":"40458","inReplyTo":"xmqq60ylv3bk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 1/4] Refactor skipping DOS drive prefixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-23T08:25:41Z","receivedAt":"2016-01-23T08:25:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 22 Jan 2016, Junio C Hamano wrote:\n\n> Johannes Sixt writes:\n> \n> > I suggest to move the function definition out of line:\n> >\n> > diff --git a/compat/mingw.c b/compat/mingw.c\n> > index 10a51c0..0cebb61 100644\n> > --- a/compat/mingw.c\n> > +++ b/compat/mingw.c\n> > @@ -1915,6 +1915,13 @@ pid_t waitpid(pid_t pid, int *status, int options)\n> > \treturn -1;\n> > }\n> > \n> > +int mingw_skip_dos_drive_prefix(char **path)\n> > +{\n> > +\tint ret = has_dos_drive_prefix(*path);\n> > +\t*path += ret;\n> > +\treturn ret;\n> > +}\n> > +\n> > int mingw_offset_1st_component(const char *path)\n> > {\n> > \tchar *pos = (char *)path;\n> > diff --git a/compat/mingw.h b/compat/mingw.h\n> > index 9b5db4e..2099b79 100644\n> > --- a/compat/mingw.h\n> > +++ b/compat/mingw.h\n> > @@ -360,12 +360,7 @@ HANDLE winansi_get_osfhandle(int fd);\n> > \n> > #define has_dos_drive_prefix(path) \\\n> > \t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n> > -static inline int mingw_skip_dos_drive_prefix(char **path)\n> > -{\n> > -\tint ret = has_dos_drive_prefix(*path);\n> > -\t*path += ret;\n> > -\treturn ret;\n> > -}\n> > +int mingw_skip_dos_drive_prefix(char **path);\n> > #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n> > #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n> > static inline char *mingw_find_last_dir_sep(const char *path)\n> \n> This sounds good to me. Dscho?\n\nYep, sounds good to me, too.\n\nPersonally, I have no inclination to add compatibility with the\nnow-safely-obsolete MSys to my responsibilities, but if Hannes wants to do\nit, who am I to stand in his way? Especially when the fix is as trivial as\nhere.\n\nCiao,\nDscho\n"},{"id":"276612","messageId":"56A3CE34.20808@kdbg.org","threadId":"40458","inReplyTo":"alpine.DEB.2.20.1601230924090.2964@virtualbox","subject":"Re: [PATCH v4 1/4] Refactor skipping DOS drive prefixes","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-01-23T19:02:12Z","receivedAt":"2016-01-23T19:02:12Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 23.01.2016 um 09:25 schrieb Johannes Schindelin:\n> Hi Junio,\n>\n> On Fri, 22 Jan 2016, Junio C Hamano wrote:\n>\n>> Johannes Sixt writes:\n>>\n>>> I suggest to move the function definition out of line:\n>>>\n>>> diff --git a/compat/mingw.c b/compat/mingw.c\n>>> index 10a51c0..0cebb61 100644\n>>> --- a/compat/mingw.c\n>>> +++ b/compat/mingw.c\n>>> @@ -1915,6 +1915,13 @@ pid_t waitpid(pid_t pid, int *status, int options)\n>>> \treturn -1;\n>>> }\n>>>\n>>> +int mingw_skip_dos_drive_prefix(char **path)\n>>> +{\n>>> +\tint ret = has_dos_drive_prefix(*path);\n>>> +\t*path += ret;\n>>> +\treturn ret;\n>>> +}\n>>> +\n>>> int mingw_offset_1st_component(const char *path)\n>>> {\n>>> \tchar *pos = (char *)path;\n>>> diff --git a/compat/mingw.h b/compat/mingw.h\n>>> index 9b5db4e..2099b79 100644\n>>> --- a/compat/mingw.h\n>>> +++ b/compat/mingw.h\n>>> @@ -360,12 +360,7 @@ HANDLE winansi_get_osfhandle(int fd);\n>>>\n>>> #define has_dos_drive_prefix(path) \\\n>>> \t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n>>> -static inline int mingw_skip_dos_drive_prefix(char **path)\n>>> -{\n>>> -\tint ret = has_dos_drive_prefix(*path);\n>>> -\t*path += ret;\n>>> -\treturn ret;\n>>> -}\n>>> +int mingw_skip_dos_drive_prefix(char **path);\n>>> #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n>>> #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n>>> static inline char *mingw_find_last_dir_sep(const char *path)\n>>\n>> This sounds good to me. Dscho?\n>\n> Yep, sounds good to me, too.\n>\n> Personally, I have no inclination to add compatibility with the\n> now-safely-obsolete MSys to my responsibilities, but if Hannes wants to do\n> it, who am I to stand in his way? Especially when the fix is as trivial as\n> here.\n\nThis is not a matter of compatibility. I am VERY curious why you do not \nsee an error (or warning) without my proposed fixup. As I mentioned, \nisalpha() is defined much later than the definition of \nmingw_skip_dos_drive_prefix(). Where does your build get a declaration \nof isalpha() from?\n\n-- Hannes\n"},{"id":"276625","messageId":"alpine.DEB.2.20.1601241152060.2964@virtualbox","threadId":"40458","inReplyTo":"56A3CE34.20808@kdbg.org","subject":"Re: [PATCH v4 1/4] Refactor skipping DOS drive prefixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-24T10:56:26Z","receivedAt":"2016-01-24T10:56:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Hannes,\n\nOn Sat, 23 Jan 2016, Johannes Sixt wrote:\n\n> Am 23.01.2016 um 09:25 schrieb Johannes Schindelin:\n>\n> > On Fri, 22 Jan 2016, Junio C Hamano wrote:\n> >\n> > > Johannes Sixt writes:\n> > >\n> > > > I suggest to move the function definition out of line:\n> > > >\n> > > > diff --git a/compat/mingw.c b/compat/mingw.c\n> > > > index 10a51c0..0cebb61 100644\n> > > > --- a/compat/mingw.c\n> > > > +++ b/compat/mingw.c\n> > > > @@ -1915,6 +1915,13 @@ pid_t waitpid(pid_t pid, int *status, int\n> > > > options)\n> > > > \treturn -1;\n> > > > }\n> > > >\n> > > > +int mingw_skip_dos_drive_prefix(char **path)\n> > > > +{\n> > > > +\tint ret = has_dos_drive_prefix(*path);\n> > > > +\t*path += ret;\n> > > > +\treturn ret;\n> > > > +}\n> > > > +\n> > > > int mingw_offset_1st_component(const char *path)\n> > > > {\n> > > > \tchar *pos = (char *)path;\n> > > > diff --git a/compat/mingw.h b/compat/mingw.h\n> > > > index 9b5db4e..2099b79 100644\n> > > > --- a/compat/mingw.h\n> > > > +++ b/compat/mingw.h\n> > > > @@ -360,12 +360,7 @@ HANDLE winansi_get_osfhandle(int fd);\n> > > >\n> > > > #define has_dos_drive_prefix(path) \\\n> > > > \t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n> > > > -static inline int mingw_skip_dos_drive_prefix(char **path)\n> > > > -{\n> > > > -\tint ret = has_dos_drive_prefix(*path);\n> > > > -\t*path += ret;\n> > > > -\treturn ret;\n> > > > -}\n> > > > +int mingw_skip_dos_drive_prefix(char **path);\n> > > > #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n> > > > #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n> > > > static inline char *mingw_find_last_dir_sep(const char *path)\n> > >\n> > > This sounds good to me. Dscho?\n> >\n> > Yep, sounds good to me, too.\n> >\n> > Personally, I have no inclination to add compatibility with the\n> > now-safely-obsolete MSys to my responsibilities, but if Hannes wants to do\n> > it, who am I to stand in his way? Especially when the fix is as trivial as\n> > here.\n> \n> This is not a matter of compatibility. I am VERY curious why you do not see\n> an error (or warning) without my proposed fixup. As I mentioned, isalpha() is\n> defined much later than the definition of mingw_skip_dos_drive_prefix().\n> Where does your build get a declaration of isalpha() from?\n\n$ grep -w isalpha /mingw32/i686-w64-mingw32/include/*.h\n/mingw32/i686-w64-mingw32/include/ctype.h: _CRTIMP int __cdecl isalpha(int _C);\n/mingw32/i686-w64-mingw32/include/ctype.h:#define __iscsymf(_c) (isalpha(_c) || ((_c)=='_'))\n\nI guess that definition gets pulled in somehow.\n\nCiao,\nDscho\n"},{"id":"276627","messageId":"56A4C534.6040503@kdbg.org","threadId":"40458","inReplyTo":"alpine.DEB.2.20.1601241152060.2964@virtualbox","subject":"Re: [PATCH v4 1/4] Refactor skipping DOS drive prefixes","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-01-24T12:36:04Z","receivedAt":"2016-01-24T12:36:04Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 24.01.2016 um 11:56 schrieb Johannes Schindelin:\n> $ grep -w isalpha /mingw32/i686-w64-mingw32/include/*.h\n> /mingw32/i686-w64-mingw32/include/ctype.h: _CRTIMP int __cdecl isalpha(int _C);\n> /mingw32/i686-w64-mingw32/include/ctype.h:#define __iscsymf(_c) (isalpha(_c) || ((_c)=='_'))\n> \n> I guess that definition gets pulled in somehow.\n\nOk, then, Junio, kindly replace js/dirname-basename~4 with this patch.\n(I hope I get the patch headers right for git-am.)\n\n---- 8< ----\nFrom: Johannes Schindelin \nSubject: [PATCH] Refactor skipping DOS drive prefixes\n\nJunio noticed that there is an implicit assumption in pretty much\nall the code calling has_dos_drive_prefix(): it forces all of its\ncallsites to hardcode the knowledge that the DOS drive prefix is\nalways two bytes long.\n\nWhile this assumption is pretty safe, we can still make the code\nmore readable and less error-prone by introducing a function that\nskips the DOS drive prefix safely.\n\nWhile at it, we change the has_dos_drive_prefix() return value: it\nnow returns the number of bytes to be skipped if there is a DOS\ndrive prefix.\n\n[j6t: moved definition of mingw_skip_dos_drive_prefix() out of line]\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Johannes Sixt \n---\n compat/basename.c | 4 +---\n compat/mingw.c | 21 ++++++++++++---------\n compat/mingw.h | 5 ++++-\n git-compat-util.h | 8 ++++++++\n path.c | 14 +++++---------\n 5 files changed, 30 insertions(+), 22 deletions(-)\n\ndiff --git a/compat/basename.c b/compat/basename.c\nindex d8f8a3c..9f00421 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -4,9 +4,7 @@\n char *gitbasename (char *path)\n {\n \tconst char *base;\n-\t/* Skip over the disk name in MSDOS pathnames. */\n-\tif (has_dos_drive_prefix(path))\n-\t\tpath += 2;\n+\tskip_dos_drive_prefix(&path);\n \tfor (base = path; *path; path++) {\n \t\tif (is_dir_sep(*path))\n \t\t\tbase = path + 1;\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex f74da23..0cebb61 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1915,28 +1915,31 @@ pid_t waitpid(pid_t pid, int *status, int options)\n \treturn -1;\n }\n \n+int mingw_skip_dos_drive_prefix(char **path)\n+{\n+\tint ret = has_dos_drive_prefix(*path);\n+\t*path += ret;\n+\treturn ret;\n+}\n+\n int mingw_offset_1st_component(const char *path)\n {\n-\tint offset = 0;\n-\tif (has_dos_drive_prefix(path))\n-\t\toffset = 2;\n+\tchar *pos = (char *)path;\n \n \t/* unc paths */\n-\telse if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {\n-\n+\tif (!skip_dos_drive_prefix(&pos) &&\n+\t\t\tis_dir_sep(pos[0]) && is_dir_sep(pos[1])) {\n \t\t/* skip server name */\n-\t\tchar *pos = strpbrk(path + 2, \"\\\\/\");\n+\t\tpos = strpbrk(pos + 2, \"\\\\/\");\n \t\tif (!pos)\n \t\t\treturn 0; /* Error: malformed unc path */\n \n \t\tdo {\n \t\t\tpos++;\n \t\t} while (*pos && !is_dir_sep(*pos));\n-\n-\t\toffset = pos - path;\n \t}\n \n-\treturn offset + is_dir_sep(path[offset]);\n+\treturn pos + is_dir_sep(*pos) - path;\n }\n \n int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 738865c..2099b79 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -358,7 +358,10 @@ HANDLE winansi_get_osfhandle(int fd);\n * git specific compatibility\n */\n \n-#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')\n+#define has_dos_drive_prefix(path) \\\n+\t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n+int mingw_skip_dos_drive_prefix(char **path);\n+#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n static inline char *mingw_find_last_dir_sep(const char *path)\n {\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 0feeae2..38397d7 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -335,6 +335,14 @@ static inline int git_has_dos_drive_prefix(const char *path)\n #define has_dos_drive_prefix git_has_dos_drive_prefix\n #endif\n \n+#ifndef skip_dos_drive_prefix\n+static inline int git_skip_dos_drive_prefix(char **path)\n+{\n+\treturn 0;\n+}\n+#define skip_dos_drive_prefix git_skip_dos_drive_prefix\n+#endif\n+\n #ifndef is_dir_sep\n static inline int git_is_dir_sep(int c)\n {\ndiff --git a/path.c b/path.c\nindex 38f2ebd..747d6da 100644\n--- a/path.c\n+++ b/path.c\n@@ -544,13 +544,10 @@ const char *relative_path(const char *in, const char *prefix,\n \telse if (!prefix_len)\n \t\treturn in;\n \n-\tif (have_same_root(in, prefix)) {\n+\tif (have_same_root(in, prefix))\n \t\t/* bypass dos_drive, for \"c:\" is identical to \"C:\" */\n-\t\tif (has_dos_drive_prefix(in)) {\n-\t\t\ti = 2;\n-\t\t\tj = 2;\n-\t\t}\n-\t} else {\n+\t\ti = j = has_dos_drive_prefix(in);\n+\telse {\n \t\treturn in;\n \t}\n \n@@ -703,11 +700,10 @@ const char *remove_leading_path(const char *in, const char *prefix)\n int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n {\n \tchar *dst0;\n+\tint i;\n \n-\tif (has_dos_drive_prefix(src)) {\n+\tfor (i = has_dos_drive_prefix(src); i > 0; i--)\n \t\t*dst++ = *src++;\n-\t\t*dst++ = *src++;\n-\t}\n \tdst0 = dst;\n \n \tif (is_dir_sep(*src)) {\n-- \n2.7.0.118.g90056ae\n"},{"id":"276678","messageId":"xmqqzivubpac.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"56A4C534.6040503@kdbg.org","subject":"Re: [PATCH v4 1/4] Refactor skipping DOS drive prefixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-24T22:12:11Z","receivedAt":"2016-01-24T22:12:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 24.01.2016 um 11:56 schrieb Johannes Schindelin:\n>> $ grep -w isalpha /mingw32/i686-w64-mingw32/include/*.h\n>> /mingw32/i686-w64-mingw32/include/ctype.h: _CRTIMP int __cdecl isalpha(int _C);\n>> /mingw32/i686-w64-mingw32/include/ctype.h:#define __iscsymf(_c) (isalpha(_c) || ((_c)=='_'))\n>> \n>> I guess that definition gets pulled in somehow.\n>\n> Ok, then, Junio, kindly replace js/dirname-basename~4 with this patch.\n> (I hope I get the patch headers right for git-am.)\n\nThanks for following it through.\n\nIf they are already in 'next', we'd want an incremental fixup, though.\n\n ... goes and looks ...\n\nYeah, unfortunately that is already in 'next'; could you make this\ninto an incremental, which would come with its own log message that\nexplains why the inlining was wrong?\n\nThanks.\n"},{"id":"276737","messageId":"56A6980C.6040701@kdbg.org","threadId":"40458","inReplyTo":"xmqqzivubpac.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] mingw: avoid linking to the C library's isalpha()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-01-25T21:47:56Z","receivedAt":"2016-01-25T21:47:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"The implementation of mingw_skip_dos_drive_prefix() calls isalpha() via\nhas_dos_drive_prefix(). Since the definition occurs long before isalpha()\nis defined in git-compat-util.h, my build environment reports:\n\n CC alloc.o\nIn file included from git-compat-util.h:186,\n from cache.h:4,\n from alloc.c:12:\ncompat/mingw.h: In function 'mingw_skip_dos_drive_prefix':\ncompat/mingw.h:365: warning: implicit declaration of function 'isalpha'\n\nDscho does not see a similar warning in his build and suspects that\nctype.h is included somehow behind the scenes. This implies that his build\nlinks to the C library's isalpha() and does not use git's isalpha().\n\nTo fix both the warning in my build and the inconsistency in Dscho's\nbuild, move the function definition to mingw.c. Then it picks up git's\nisalpha() because git-compat-util.h is included at the top of the file.\n\nSigned-off-by: Johannes Sixt \n---\n compat/mingw.c | 7 +++++++\n compat/mingw.h | 7 +------\n 2 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 10a51c0..0cebb61 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1915,6 +1915,13 @@ pid_t waitpid(pid_t pid, int *status, int options)\n \treturn -1;\n }\n \n+int mingw_skip_dos_drive_prefix(char **path)\n+{\n+\tint ret = has_dos_drive_prefix(*path);\n+\t*path += ret;\n+\treturn ret;\n+}\n+\n int mingw_offset_1st_component(const char *path)\n {\n \tchar *pos = (char *)path;\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 9b5db4e..2099b79 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -360,12 +360,7 @@ HANDLE winansi_get_osfhandle(int fd);\n \n #define has_dos_drive_prefix(path) \\\n \t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n-static inline int mingw_skip_dos_drive_prefix(char **path)\n-{\n-\tint ret = has_dos_drive_prefix(*path);\n-\t*path += ret;\n-\treturn ret;\n-}\n+int mingw_skip_dos_drive_prefix(char **path);\n #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix\n #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n static inline char *mingw_find_last_dir_sep(const char *path)\n-- \n2.7.0.118.g90056ae\n"},{"id":"276741","messageId":"xmqq60yhfh8v.fsf@gitster.mtv.corp.google.com","threadId":"40458","inReplyTo":"56A6980C.6040701@kdbg.org","subject":"Re: [PATCH] mingw: avoid linking to the C library's isalpha()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-25T22:04:32Z","receivedAt":"2016-01-25T22:04:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"}]}