Re: [PATCH 2/4] submodule: implement `module_name` as a builtin helper

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Aug 4, 2015 at 8:04 PM, Stefan Beller <sbeller@google.com> wrote:
> The goal of this series being rewriting `git submodule update`,
> we don't want to call out to the shell script for config lookups.
>
> So reimplement the lookup of the submodule name in C.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index cb18ddf..dd5635f 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -98,6 +100,48 @@ static int module_list(int argc, const char **argv, const char *prefix)
> +static int collect_module_names(const char *key, const char *value, void *cb)
> +{
> +       size_t len;
> +       struct string_list *sl = cb;
> +
> +       if (starts_with(key, "submodule.")
> +           && strip_suffix(key, ".path", &len)) {
> +               struct strbuf sb = STRBUF_INIT;
> +               strbuf_add(&sb, key + strlen("submodule."),
> +                               len - strlen("submodule."));
> +               string_list_insert(sl, value)->util = strbuf_detach(&sb, NULL);
> +               strbuf_release(&sb);

Why the complexity and overhead of a strbuf when the same could be
accomplished more easily and straightforwardly with xstrndup()?

> +       }
> +
> +       return 0;
> +}
> +
> +static int module_name(int argc, const char **argv, const char *prefix)
> +{
> +       struct string_list_item *item;
> +       struct git_config_source config_source;
> +       struct string_list values = STRING_LIST_INIT_DUP;
> +
> +       if (!argc)

Do you mean?

    if (argc != 1)

> +               usage("git submodule--helper module_name <path>\n");
> +
> +       memset(&config_source, 0, sizeof(config_source));
> +       config_source.file = ".gitmodules";
> +
> +       if (git_config_with_options(collect_module_names, &values,
> +                                   &config_source, 1) < 0)
> +               die(_("unknown error occured while reading the git modules file"));
> +
> +       item = string_list_lookup(&values, argv[0]);
> +       if (item)
> +               printf("%s\n", (char*)item->util);
> +       else
> +               die("No submodule mapping found in .gitmodules for path '%s'", argv[0]);
> +       return 0;
> +}
> +
>  int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
>  {
>         if (argc < 2)
> @@ -106,6 +150,9 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
>         if (!strcmp(argv[1], "module_list"))
>                 return module_list(argc - 1, argv + 1, prefix);
>
> +       if (!strcmp(argv[1], "module_name"))
> +               return module_name(argc - 2, argv + 2, prefix);
> +
>  usage:
>         usage("git submodule--helper module_list\n");
>  }