[PATCH v2 22/28] submodule--helper: move submodule_strategy_to_string() to only user
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Aug 2, 2022, 15:46 UTC
- Message-ID
- <patch-v2-22.28-98c3e562c82-20220802T154036Z-avarab@gmail.com>
- In-Reply-To
- <cover-v2-00.28-00000000000-20220802T154036Z-avarab@gmail.com>
Move the submodule_strategy_to_string() function added in 3604242f080 (submodule: port init from shell to C, 2016-04-15) to its only user.
This function would return NULL on SM_UPDATE_UNSPECIFIED, so it wasn't safe to xstrdup() its return value in the general case, or to use it in a sprintf() format as the code removed in the preceding commit did.
But its callers would never call it with either SM_UPDATE_UNSPECIFIED or SM_UPDATE_COMMAND. Let's move it to a "static" helper, and have its functionality reflect how it's used, and BUG() out on the rest.
By doing this we can also stop needlessly xstrdup()-ing and free()-ing the memory for the config we're setting. We can instead always use constant strings. We can also use the *_tmp() variant of git_config_get_string().
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/submodule--helper.c | 29 ++++++++++++++++++++++++----- submodule.c | 21 --------------------- submodule.h | 1 - 3 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index b49528e1ba9..d787c0fead4 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -413,12 +413,32 @@ struct init_cb { }; #define INIT_CB_INIT { 0 } +static const char *submodule_strategy_to_string(enum submodule_update_type type) +{ + switch (type) { + case SM_UPDATE_CHECKOUT: + return "checkout"; + case SM_UPDATE_MERGE: + return "merge"; + case SM_UPDATE_REBASE: + return "rebase"; + case SM_UPDATE_NONE: + return "none"; + case SM_UPDATE_UNSPECIFIED: + case SM_UPDATE_COMMAND: + BUG("init_submodule() should handle type %d", type); + default: + BUG("unexpected update strategy type: %d", type); + } +} + static void init_submodule(const char *path, const char *prefix, unsigned int flags) { const struct submodule *sub; struct strbuf sb = STRBUF_INIT; - char *upd = NULL, *url = NULL, *displaypath; + const char *upd; + char *url = NULL, *displaypath; displaypath = get_submodule_displaypath(path, prefix); @@ -474,14 +494,14 @@ static void init_submodule(const char *path, const char *prefix, /* Copy "update" setting when it is not set yet */ strbuf_addf(&sb, "submodule.%s.update", sub->name); - if (git_config_get_string(sb.buf, &upd) && + if (git_config_get_string_tmp(sb.buf, &upd) && sub->update_strategy.type != SM_UPDATE_UNSPECIFIED) { if (sub->update_strategy.type == SM_UPDATE_COMMAND) { fprintf(stderr, _("warning: command update mode suggested for submodule '%s'\n"), sub->name); - upd = xstrdup("none"); + upd = "none"; } else { - upd = xstrdup(submodule_strategy_to_string(&sub->update_strategy)); + upd = submodule_strategy_to_string(sub->update_strategy.type); } if (git_config_set_gently(sb.buf, upd)) @@ -490,7 +510,6 @@ static void init_submodule(const char *path, const char *prefix, strbuf_release(&sb); free(displaypath); free(url); - free(upd); } static void init_submodule_cb(const struct cache_entry *list_item, void *cb_data) diff --git a/submodule.c b/submodule.c index 3fa5db3ecdf..e297d94d17f 100644 --- a/submodule.c +++ b/submodule.c @@ -415,27 +415,6 @@ int parse_submodule_update_strategy(const char *value, return 0; } -const char *submodule_strategy_to_string(const struct submodule_update_strategy *s) -{ - struct strbuf sb = STRBUF_INIT; - switch (s->type) { - case SM_UPDATE_CHECKOUT: - return "checkout"; - case SM_UPDATE_MERGE: - return "merge"; - case SM_UPDATE_REBASE: - return "rebase"; - case SM_UPDATE_NONE: - return "none"; - case SM_UPDATE_UNSPECIFIED: - return NULL; - case SM_UPDATE_COMMAND: - strbuf_addf(&sb, "!%s", s->command); - return strbuf_detach(&sb, NULL); - } - return NULL; -} - void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *arg) { diff --git a/submodule.h b/submodule.h index bfaa9da1868..949f67b8031 100644 --- a/submodule.h +++ b/submodule.h @@ -72,7 +72,6 @@ void die_path_inside_submodule(struct index_state *istate, enum submodule_update_type parse_submodule_update_type(const char *value); int parse_submodule_update_strategy(const char *value, struct submodule_update_strategy *dst); -const char *submodule_strategy_to_string(const struct submodule_update_strategy *s); void handle_ignore_submodules_arg(struct diff_options *, const char *); void show_submodule_diff_summary(struct diff_options *o, const char *path, struct object_id *one, struct object_id *two,
-- 2.37.1.1233.ge8b09efaedc