On Wed, Oct 28, 2015 at 7:21 PM, Stefan Beller <sbel...@google.com> wrote:
> submodule config: remove name_and_item_from_var
>
> By inlining `name_and_item_from_var` it is easy to add later options
> which are not required to have a submodule name.
I guess you're trying to say that name_and_item_from_var() didn't
provide a proper abstraction, thus wasn't as useful as expected.
Perhaps that commit message could make this shortcoming clearer.
> Signed-off-by: Stefan Beller <sbel...@google.com>
> ---
> diff --git a/submodule-config.c b/submodule-config.c
> index 8b8c7d1..4d0563c 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -251,18 +235,25 @@ static int parse_config(const char *var, const char
> *value, void *data)
> {
> struct parse_config_parameter *me = data;
> struct submodule *submodule;
> - struct strbuf name = STRBUF_INIT, item = STRBUF_INIT;
> - int ret = 0;
> + int subsection_len, ret = 0;
> + const char *subsection, *key;
> + char *name;
>
> - /* this also ensures that we only parse submodule entries */
> - if (!name_and_item_from_var(var, &name, &item))
> + if (parse_config_key(var, "submodule", &subsection,
> + &subsection_len, &key) < 0)
> return 0;
>
> + if (!subsection_len)
> + return 0;
Alternately:
if (parse_config_key(var, "submodule", &subsection,
&subsection_len, &key) < 0 || !subsection_len)
return 0;
> +
> + /* subsection is not null terminated */
> + name = xmemdupz(subsection, subsection_len);
> submodule = lookup_or_create_by_name(me->cache,
> me->gitmodules_sha1,
> - name.buf);
> + name);
> + free(name);
Since this is all private to submodule-config.c, I wonder if it would
be cleaner to change lookup_or_create_by_name() to accept a
name_length argument?
> - if (!strcmp(item.buf, "path")) {
> + if (!strcmp(key, "path")) {
> if (!value)
> ret = config_error_nonbool(var);
> else if (!me->overwrite && submodule->path != NULL)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
