Re: [PATCH v4] clone: simplify string handling in guess_dir_name()

From: Lukas Fleischer <lfleischer@lfos.de>
Date: Aug 4, 2015, 04:34 UTC
Message-ID: <20150804043401.4494.43725@typhoon>
In-Reply-To: <0000014e740f7a8a-2c988a36-633e-4b30-8024-cb4a1de1a8a2-000000@eu-west-1.amazonses.com>

On Thu, 09 Jul 2015 at 20:24:08, Sebastian Schuberth wrote:

Show 30 quoted lines

> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
> ---
>  builtin/clone.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 00535d0..ebcb849 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -147,6 +147,7 @@ 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;
>         char *dir;
>  
>         /*
> @@ -173,19 +174,11 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
>         /*
>          * Strip .{bundle,git}.
>          */
> -       if (is_bundle) {
> -               if (end - start > 7 && !strncmp(end - 7, ".bundle", 7))
> -                       end -= 7;
> -       } else {
> -               if (end - start > 4 && !strncmp(end - 4, ".git", 4))
> -                       end -= 4;
> -       }
> +       strip_suffix(start, is_bundle ? ".bundle" : ".git" , &len);
> [...]

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?