On Tue, Aug 04, 2015 at 09:31:18AM +0200, Sebastian Schuberth wrote: > On Tue, Aug 4, 2015 at 6:34 AM, Lukas Fleischer <lfleischer@xxxxxxx> wrote: > > > I am currently on vacation and cannot bisect or debug this but I am > > pretty confident that this patch changes the behaviour of directory name > > guessing. With Git 2.4.6, cloning http://foo.bar/foo.git/ results in a > > directory named foo and with Git 2.5.0, the resulting directory is > > called foo.git. > > > > Note how the end variable is decreased when the repository name ends > > with a slash but that isn't taken into account when simply using > > strip_suffix() later... > > > > Is this intended? > > I did not intend this change in behavior, and I can confirm that > reverting my patch restores the original behavior. Thanks for bringing > this to my attention, I'll work on a patch. I think this regression is in v2.4.8, as well. We should be able to use a running "len" instead of the "end" pointer in the earlier part, and then use strip_suffix_mem later (to strip from our already-reduced length, rather than the full NUL-terminated string). Like this: diff --git a/builtin/clone.c b/builtin/clone.c index 303a3a7..4b61e4c 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -146,20 +146,19 @@ static char *get_repo_path(const char *repo, int *is_bundle) static char *guess_dir_name(const char *repo, int is_bundle, int is_bare) { - const char *end = repo + strlen(repo), *start; - size_t len; + const char *start; + size_t len = strlen(repo); char *dir; /* * Strip trailing spaces, slashes and /.git */ - while (repo < end && (is_dir_sep(end[-1]) || isspace(end[-1]))) - end--; - if (end - repo > 5 && is_dir_sep(end[-5]) && - !strncmp(end - 4, ".git", 4)) { - end -= 5; - while (repo < end && is_dir_sep(end[-1])) - end--; + while (len > 0 && (is_dir_sep(repo[len-1]) || isspace(repo[len-1]))) + len--; + if (len > 5 && is_dir_sep(repo[len-5]) && + strip_suffix_mem(repo, &len, ".git")) { + while (len > 0 && is_dir_sep(repo[len-1])) + len--; } /* @@ -167,14 +166,14 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare) * the form "remote.example.com:foo.git", i.e. no slash * in the directory part. */ - start = end; + start = repo + len; while (repo < start && !is_dir_sep(start[-1]) && start[-1] != ':') start--; /* * Strip .{bundle,git}. */ - strip_suffix(start, is_bundle ? ".bundle" : ".git" , &len); + strip_suffix_mem(start, &len, is_bundle ? ".bundle" : ".git"); if (is_bare) dir = xstrfmt("%.*s.git", (int)len, start); @@ -187,6 +186,7 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare) if (*dir) { char *out = dir; int prev_space = 1 /* strip leading whitespace */; + const char *end; for (end = dir; *end; ++end) { char ch = *end; if ((unsigned char)ch < '\x20') Sadly we cannot just `strip_suffix_mem(repo, &len, "/.git"))` in the earlier code, as we have to account for multiple directory separators. I believe the above code does the right thing, though. I haven't looked at how badly it interacts with the other guess_dir_name work from Patrick Steinhardt that has been going on, though. -Peff -- 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