Re: [PATCH 2/4] submodule--helper clone: simplify path check

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 31, 2016, 07:31 UTC
Message-ID: <CAPig+cTz3fTyXFM4oPNO3Tv6dP0hN152jf9_JZOrsAZad1+O6Q@mail.gmail.com>
In-Reply-To: <1459383457-6848-3-git-send-email-sbeller@google.com>

On Wed, Mar 30, 2016 at 8:17 PM, Stefan Beller <sbeller@google.com> wrote:

> The calling shell code makes sure that `path` is non null and non empty.
> (side note: it cannot be null as just three lines before it is passed
> to safe_create_leading_directories_const which would crash as you feed
> it null).

I'm confused by this. So, you're saying that it's okay (and desirable) for git-submodule--helper to segfault when the user does this:

    % git submodule--helper clone
    Segmentation fault: 11

rather than, say, printing a useful error message, such as:

    submodule--helper: missing or empty --path

Show 16 quoted lines

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> @@ -215,10 +215,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>         if (safe_create_leading_directories_const(path) < 0)
>                 die(_("could not create directory '%s'"), path);
>
> -       if (path && *path)
> -               strbuf_addf(&sb, "%s/.git", path);
> -       else
> -               strbuf_addstr(&sb, ".git");
> +       strbuf_addf(&sb, "%s/.git", path);
>
>         if (safe_create_leading_directories_const(sb.buf) < 0)
>                 die(_("could not create leading directories of '%s'"), sb.buf);
> --