[PATCH v2 22/28] submodule--helper: move submodule_strategy_to_string() to only user

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
---
 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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux