Hi,
On Mon, 25 Jan 2010, Robin Rosenberg wrote:
> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
> From: Robin Rosenberg <robin.rosenberg@xxxxxxxxxx>
> Date: Mon, 25 Jan 2010 01:41:03 +0100
> Subject: [PATCH] Handle UNC paths everywhere
>
> In Windows paths beginning with // are knows as UNC paths. They are
> absolute paths, usually referring to a shared resource on a server.
And even a simple "cd" with them does not work.
> Examples of legal UNC paths
>
> \\hub\repos\repo
> \\?\unc\hub\repos
> \\?\d:\repo
>
> Signed-off-by: Robin Rosenberg <robin.rosenberg@xxxxxxxxxx>
> ---
> cache.h | 2 +-
> compat/basename.c | 2 +-
> compat/mingw.h | 8 +++++++-
> connect.c | 2 +-
> git-compat-util.h | 9 +++++++++
> path.c | 2 +-
> setup.c | 2 +-
> sha1_file.c | 20 ++++++++++++++++++++
> transport.c | 2 +-
> 9 files changed, 42 insertions(+), 7 deletions(-)
Ouch. You should know better than to clutter non-Windows-specific parts
with that ugly kludge.
> diff --git a/cache.h b/cache.h
> index 767a50e..8f63640 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char
> *path);
> char *enter_repo(char *path, int strict);
> static inline int is_absolute_path(const char *path)
> {
> - return path[0] == '/' || has_dos_drive_prefix(path);
> + return path[0] == '/' || has_win32_abs_prefix(path);
Why? We can still keep the name. Well, maybe not, see below.
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 1b528da..d1aa8be 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -210,7 +210,13 @@ int winansi_fprintf(FILE *stream, const char *format,
> ...) __attribute__((format
> * git specific compatibility
> */
>
> -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')
> +#define has_dos_drive_prefix(path) \
> + (isalpha(*(path)) && (path)[1] == ':')
Why?
> +#define has_unc_prefix(path) \
> + (is_dir_sep((path)[0]) && is_dir_sep((path)[1]))
> +#define has_win32_abs_prefix(path) \
> + (has_dos_drive_prefix(path) || has_unc_prefix(path))
"c:hello.txt" is not an absolute path.
> diff --git a/connect.c b/connect.c
> index 7945e38..9d4556c 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const char
> *url_orig,
> end = host;
>
> path = strchr(end, c);
> - if (path && !has_dos_drive_prefix(end)) {
> + if (path && !has_win32_abs_prefix(end)) {
> if (c == ':') {
Why? Do we really have to exclude UNC paths from that ":" handling?
> diff --git a/git-compat-util.h b/git-compat-util.h
> index ef60803..0de9dac 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -170,6 +170,15 @@ extern char *gitbasename(char *);
> #define has_dos_drive_prefix(path) 0
> #endif
>
> +#ifndef has_unc_prefix
> +#define has_unc_prefix(path) 0
> +#endif
> +
> +#ifndef has_win32_abs_prefix
> +#error no abs
Yeah, sure. I do have abs, thank you very much.
In general, I am _very_ worried about your patch. It does not acknowledge
that there is a fundamental difference between DOS drive prefixes and UNC
paths, and not being able to "cd" to the latter is just a symptom.
I am also not quite sure if you can get away with having the same offset
for both: if I have "C:\blah" and strip off "C:", I always have a
directory separator to bounce against, whereas I do not have that if I
strip off the two "\\" of a UNC path. Besides, I maintain that the host
name, and maybe even the share name, should not ever be stripped off!
At least a discussion is missing from the commit message.
Ciao,
Dscho
--
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