[PATCH] Make mingw_offset_1st_component() behave consistently for all paths.

[Eric Sunshine]

mingw_offset_1st_component() returns "foo" for inputs "/foo" and
"c:/foo", but inconsistently returns "/foo" for UNC input
"/machine/share/foo". Fix it to return "foo" for all cases.

Reference: http://groups.google.com/group/msysgit/browse_thread/thread/c0af578549b5dda0

Signed-off-by: Eric Sunshine <suns...@sunshineco.com>
---
compat/mingw.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 861d3da..81c8b2e 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1747,23 +1747,24 @@ const char *get_windows_home_directory()

int mingw_offset_1st_component(const char *path)
{
+ int offset = 0;
if (has_dos_drive_prefix(path))
- return 2 + is_dir_sep(path[2]);
+ offset = 2;

/* unc paths */
- if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {
+ else if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {

/* skip server name */
char *pos = strpbrk(path + 2, "\\/");
if (!pos)
- return 0;
+ return 0; /* Error: malformed unc path */

do {
pos++;
} while (*pos && !is_dir_sep(*pos));

- return pos - path;
+ offset = pos - path;
}

- return is_dir_sep(path[0]);
+ return offset + is_dir_sep(path[offset]);
}
--
1.7.1.msysgit.0.385.gff43b.dirty

[Johannes Schindelin]

Hi,

On Tue, 13 Jul 2010, Eric Sunshine wrote:

> mingw_offset_1st_component() returns "foo" for inputs "/foo" and
> "c:/foo", but inconsistently returns "/foo" for UNC input
> "/machine/share/foo". Fix it to return "foo" for all cases.
>
> Reference: http://groups.google.com/group/msysgit/browse_thread/thread/c0af578549b5dda0
>
> Signed-off-by: Eric Sunshine <suns...@sunshineco.com>

Wow, that was quick. Did you verify that it's still able to do "git init
//somewhere/foo"?

Ciao,
Dscho

[Eric Sunshine]

Yes, I checked 'git init' with paths "//unc/path/foo", "c:/foo", "/foo",
and "foo".

-- ES

[Johannes Schindelin]

Hi,

Thank you so much, Eric. I committed and pushed your changes (hopefully to
the correct repository this time :-)

Ciao,
Dscho

[Erik Faye-Lund]

It seems it's still not working 100%, though. If you're doing
something like "git init //unc/path/some/nonexistent/directories" it
seems to fail currently. At least it does so before the patch, it
looks to me like the logic is flawed.

I'm currently investigating further - the following seems to give much
more senseful results, and looks more correct to my untrained eyes...

diff --git a/compat/mingw.c b/compat/mingw.c
index 861d3da..7b949d4 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1758,11 +1758,7 @@ int mingw_offset_1st_component(const char *path)
if (!pos)
return 0;

- do {
- pos++;
- } while (*pos && !is_dir_sep(*pos));

-
- return pos - path;

+ return pos - path + 1;
}

return is_dir_sep(path[0]);

--
Erik "kusma" Faye-Lund

[Eric Sunshine]

On 7/13/2010 12:47 PM, Erik Faye-Lund wrote:
> On Tue, Jul 13, 2010 at 6:40 PM, Eric Sunshine<suns...@sunshineco.com> wrote:
>> On 7/13/2010 12:35 PM, Johannes Schindelin wrote:
>>> On Tue, 13 Jul 2010, Eric Sunshine wrote:
>>>> mingw_offset_1st_component() returns "foo" for inputs "/foo" and
>>>> "c:/foo", but inconsistently returns "/foo" for UNC input
>>>> "/machine/share/foo". Fix it to return "foo" for all cases.

>>> Wow, that was quick. Did you verify that it's still able to do "git init
>>> //somewhere/foo"?
>> Yes, I checked 'git init' with paths "//unc/path/foo", "c:/foo", "/foo", and
>> "foo".
> It seems it's still not working 100%, though. If you're doing
> something like "git init //unc/path/some/nonexistent/directories" it
> seems to fail currently. At least it does so before the patch, it
> looks to me like the logic is flawed.

Without the patch, the above does fail. With the patch, it works in my
tests:

$ git init '//localhost/c$/Users/me/Desktop/non/existent/directory'
Initialized empty Git repository in
//localhost/c$/Users/me/Desktop/non/existent/directory/.git/
$

-- ES

[Erik Faye-Lund]

It does indeed. Thank you for correcting me.

But I've found another issue while debugging: using backwards slashes
does not work when the folders needs to be created, because
safe_create_leading_directories() assumes forward-slash directory
separator. I'm about to submit a patch for this now.

--
Erik "kusma" Faye-Lund