submodule API: don't handle SM_..{UNSPECIFIED,COMMAND} in to_string()

Change the submodule_strategy_to_string() function added in 3604242f080 (submodule: port init from shell to C, 2016-04-15) to really return a "const char *". In the "SM_UPDATE_COMMAND" case it would return a strbuf_detach(). Furthermore, 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 have its behavior reflect how its only user expects it to behave, 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(). Let's also rename this submodule_strategy_to_string() function to submodule_update_type_to_string(). Now that it's only tasked with returning a string version of the "enum submodule_update_type type". Before it would look at the "command" field in "struct submodule_update_strategy". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Sep 1, 2022 at 01:18 UTC b9dd63ffe2842fe676f54d4d5f8f06bc04c6dd8b
3 files changed +11 -13
builtin/submodule--helper.c
+5 -5
@@ -418,7 +418,8 @@ static void init_submodule(const char *path, const char *prefix,
418 {
419 const struct submodule *sub;
420 struct strbuf sb = STRBUF_INIT;
421 - char *upd = NULL, *url = NULL, *displaypath;
421 + const char *upd;
422 + char *url = NULL, *displaypath;
423
424 displaypath = get_submodule_displaypath(path, prefix);
425
@@ -474,14 +475,14 @@ static void init_submodule(const char *path, const char *prefix,
475
476 /* Copy "update" setting when it is not set yet */
477 strbuf_addf(&sb, "submodule.%s.update", sub->name);
477 - if (git_config_get_string(sb.buf, &upd) &&
478 + if (git_config_get_string_tmp(sb.buf, &upd) &&
479 sub->update_strategy.type != SM_UPDATE_UNSPECIFIED) {
480 if (sub->update_strategy.type == SM_UPDATE_COMMAND) {
481 fprintf(stderr, _("warning: command update mode suggested for submodule '%s'\n"),
482 sub->name);
482 - upd = xstrdup("none");
483 + upd = "none";
484 } else {
484 - upd = xstrdup(submodule_strategy_to_string(&sub->update_strategy));
485 + upd = submodule_update_type_to_string(sub->update_strategy.type);
486 }
487
488 if (git_config_set_gently(sb.buf, upd))
@@ -490,7 +491,6 @@ static void init_submodule(const char *path, const char *prefix,
491 strbuf_release(&sb);
492 free(displaypath);
493 free(url);
493 - free(upd);
494 }
495
496 static void init_submodule_cb(const struct cache_entry *list_item, void *cb_data)
submodule.c
+5 -7
@@ -415,10 +415,9 @@ int parse_submodule_update_strategy(const char *value,
415 return 0;
416 }
417
418 -const char *submodule_strategy_to_string(const struct submodule_update_strategy *s)
418 +const char *submodule_update_type_to_string(enum submodule_update_type type)
419 {
420 - struct strbuf sb = STRBUF_INIT;
421 - switch (s->type) {
420 + switch (type) {
421 case SM_UPDATE_CHECKOUT:
422 return "checkout";
423 case SM_UPDATE_MERGE:
@@ -428,12 +427,11 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy
427 case SM_UPDATE_NONE:
428 return "none";
429 case SM_UPDATE_UNSPECIFIED:
431 - return NULL;
430 case SM_UPDATE_COMMAND:
433 - strbuf_addf(&sb, "!%s", s->command);
434 - return strbuf_detach(&sb, NULL);
431 + BUG("init_submodule() should handle type %d", type);
432 + default:
433 + BUG("unexpected update strategy type: %d", type);
434 }
436 - return NULL;
435 }
436
437 void handle_ignore_submodules_arg(struct diff_options *diffopt,
submodule.h
+1 -1
@@ -72,7 +72,7 @@ void die_path_inside_submodule(struct index_state *istate,
72 enum submodule_update_type parse_submodule_update_type(const char *value);
73 int parse_submodule_update_strategy(const char *value,
74 struct submodule_update_strategy *dst);
75 -const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
75 +const char *submodule_update_type_to_string(enum submodule_update_type type);
76 void handle_ignore_submodules_arg(struct diff_options *, const char *);
77 void show_submodule_diff_summary(struct diff_options *o, const char *path,
78 struct object_id *one, struct object_id *two,