Stefan Beller <sbeller@xxxxxxxxxx> writes: > @@ -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? -- 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