Re: [PATCHv2 6/8] submodule: simplify memory handling in config parsing
From: Jens Lehmann <hidden>
Date: 2016-06-15 22:55:52
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 <redacted>
quoted hunk ↗ jump to hunk
Signed-off-by: Jeff King <redacted> --- 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; }