Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> I stumbled over the compile warning when upgrading Git for Windows
> to 2.6.0. There was a left-over NO_LIBGEN_H=YesPlease (which we
> no longer need in Git for Windows 2.x), but it did point to the
> fact that we use `dirname()` in builtin/am.c now, so we better
> have a fall-back implementation for platforms without libgen.h.
Thanks for being careful.
>
> I tested this implementation a bit, but I still would appreciate
> a few eye-balls to go over it.
>
> compat/basename.c | 26 ++++++++++++++++++++++++++
> git-compat-util.h | 2 ++
> 2 files changed, 28 insertions(+)
>
> diff --git a/compat/basename.c b/compat/basename.c
> index d8f8a3c..10dba38 100644
> --- a/compat/basename.c
> +++ b/compat/basename.c
> @@ -13,3 +13,29 @@ char *gitbasename (char *path)
> }
> return (char *)base;
> }
> +
> +char *gitdirname(char *path)
> +{
> + char *p = path, *slash, c;
> +
> + /* Skip over the disk name in MSDOS pathnames. */
> + if (has_dos_drive_prefix(p))
> + p += 2;
Not a new problem, but many callers of has_dos_drive_prefix()
hardcodes that "2" in various forms. I wonder if this is something
we should relieve callers of by tweaking the semantics of it, e.g.
by returning 2 (or howmanyever bytes should be skipped) from the
function, changing it to skip_dos_drive_prefix(&p), etc.
> + /* POSIX.1-2001 says dirname("/") should return "/" */
> + slash = is_dir_sep(*p) ? ++p : NULL;
> + while ((c = *(p++)))
I am confused by this. What is the invariant on 'p' at the
beginning of the body of this while loop in each iteration?
Inside the body, p skips over dir-sep characters, so p must point at
the byte past the last run of slashes?
If that is the invariant, upon entry, shouldn't the initialization
of "slash" be skipping over all slashes, not just the first one,
when the input is "///foo", for example? Instead the above skips '/'
and sets slash to the byte past the first '/' (which is OK because
you want to NUL-terminate to remove "//foo" from the input) but does
not move p to 'f', so the invariant is not "p must point at the byte
past the last run of slashes".
> + if (is_dir_sep(c)) {
> + char *tentative = p - 1;
> +
> + /* POSIX.1-2001 says to ignore trailing slashes */
> + while (is_dir_sep(*p))
> + p++;
> + if (*p)
> + slash = tentative;
> + }
I would have expected the function to scan from the end/right/tail.
> + if (!slash)
> + return ".";
> + *slash = '\0';
> + return path;
> +}
> diff --git a/git-compat-util.h b/git-compat-util.h
> index f649e81..8b01aa5 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -253,6 +253,8 @@ struct itimerval {
> #else
> #define basename gitbasename
> extern char *gitbasename(char *);
> +#define dirname gitdirname
> +extern char *gitdirname(char *);
> #endif
>
> #ifndef NO_ICONV
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html