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

> 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");
>>  }