Re: [PATCHv11 1/7] submodule-config: keep update strategy around
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:08:18
Stefan Beller [off-list ref] writes:
quoted hunk
@@ -340,6 +342,16 @@ static int parse_config(const char *var, const char *value, void *data) free((void *) submodule->url); submodule->url = xstrdup(value); } + } else if (!strcmp(item.buf, "update")) { + if (!value) + ret = config_error_nonbool(var); + else if (!me->overwrite && + submodule->update_strategy.type != SM_UPDATE_UNSPECIFIED) + warn_multiple_config(me->commit_sha1, submodule->name, + "update"); + else if (parse_submodule_update_strategy(value, + &submodule->update_strategy) < 0) + die(_("invalid value for %s"), var);
Mental note. This takes "value" that comes from the config API; the pre-context in this hunk treats it as a transient piece of memory and uses xstrdup() on it before storing it away in submodule->url.
+int parse_submodule_update_strategy(const char *value,
+ struct submodule_update_strategy *dst)
+{
+ dst->command = NULL;
+ if (!strcmp(value, "none"))
+ dst->type = SM_UPDATE_NONE;
+ else if (!strcmp(value, "checkout"))
+ dst->type = SM_UPDATE_CHECKOUT;
+ else if (!strcmp(value, "rebase"))
+ dst->type = SM_UPDATE_REBASE;
+ else if (!strcmp(value, "merge"))
+ dst->type = SM_UPDATE_MERGE;
+ else if (skip_prefix(value, "!", &dst->command))
+ dst->type = SM_UPDATE_COMMAND;
This however uses skip_prefix() on value, making dst->command store
a pointer into that transient piece of memory.
That looks inconsistent, doesn't it? I think this part should be
else if (value[0] == '!') {
dst->type = SM_UPDATE_COMMAND;
dst->command = xstrdup(value + 1);
}
or something like that. I.e. dst->command should own the piece of
memory it points at, no?