Am 23.01.2013 07:26, schrieb Jeff King:
> We keep a strbuf for the name of the submodule, even though
> we only ever add one string to it. Let's just use xmemdupz
> instead, which is slightly more efficient and makes it
> easier to follow what is going on.
>
> Unfortunately, we still end up having to deal with some
> memory ownership issues in some code branches, as we have to
> allocate the string in order to do a string list lookup, and
> then only sometimes want to hand ownership of that string
> over to the string_list. Still, making that explicit in the
> code (as opposed to sometimes detaching the strbuf, and then
> always releasing it) makes it a little more obvious what is
> going on.
Thanks, this helps until I some day find the time to refactor
that code into a more digestible shape ;-)
Acked-by: Jens Lehmann <Jens.Lehmann@xxxxxx>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> submodule.c | 30 ++++++++++++++----------------
> 1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 25413de..9ba1496 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -127,7 +127,6 @@ int parse_submodule_config_option(const char *var, const char *value)
> int parse_submodule_config_option(const char *var, const char *value)
> {
> struct string_list_item *config;
> - struct strbuf submodname = STRBUF_INIT;
> const char *name, *key;
> int namelen;
>
> @@ -135,37 +134,36 @@ int parse_submodule_config_option(const char *var, const char *value)
> return 0;
>
> if (!strcmp(key, "path")) {
> - strbuf_add(&submodname, name, namelen);
> config = unsorted_string_list_lookup(&config_name_for_path, value);
> if (config)
> free(config->util);
> else
> config = string_list_append(&config_name_for_path, xstrdup(value));
> - config->util = strbuf_detach(&submodname, NULL);
> - strbuf_release(&submodname);
> + config->util = xmemdupz(name, namelen);
> } else if (!strcmp(key, "fetchrecursesubmodules")) {
> - strbuf_add(&submodname, name, namelen);
> - config = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, submodname.buf);
> + char *name_cstr = xmemdupz(name, namelen);
> + config = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name_cstr);
> if (!config)
> - config = string_list_append(&config_fetch_recurse_submodules_for_name,
> - strbuf_detach(&submodname, NULL));
> + config = string_list_append(&config_fetch_recurse_submodules_for_name, name_cstr);
> + else
> + free(name_cstr);
> config->util = (void *)(intptr_t)parse_fetch_recurse_submodules_arg(var, value);
> - strbuf_release(&submodname);
> } else if (!strcmp(key, "ignore")) {
> + char *name_cstr;
> +
> if (strcmp(value, "untracked") && strcmp(value, "dirty") &&
> strcmp(value, "all") && strcmp(value, "none")) {
> warning("Invalid parameter \"%s\" for config option \"submodule.%s.ignore\"", value, var);
> return 0;
> }
>
> - strbuf_add(&submodname, name, namelen);
> - config = unsorted_string_list_lookup(&config_ignore_for_name, submodname.buf);
> - if (config)
> + name_cstr = xmemdupz(name, namelen);
> + config = unsorted_string_list_lookup(&config_ignore_for_name, name_cstr);
> + if (config) {
> free(config->util);
> - else
> - config = string_list_append(&config_ignore_for_name,
> - strbuf_detach(&submodname, NULL));
> - strbuf_release(&submodname);
> + free(name_cstr);
> + } else
> + config = string_list_append(&config_ignore_for_name, name_cstr);
> config->util = xstrdup(value);
> return 0;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html