We keep a strbuf for the name of the submodule, even though
we only ever add one buffer (which we know the length of) 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.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
I'm undecided on this one. I think the result is easier to follow, but
others might find the original easier. This would be a lot more
readable if the config parser gave us a broken-out representation with
real C strings. Then we could pass the subsection straight to the
string_list functions for lookup, and using the "dup" flag of string
list, avoid even having to deal with memory duplication at all.
submodule.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/submodule.c b/submodule.c
index 4361207..23a8490 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;
@@ -136,37 +135,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;
}
--
1.8.1.rc1.10.g7d71f7b
--
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