@matheusbsbs / matheus-bagliano / commits / a78b462976

config: clarify memory ownership when preparing comment strings

The ownership of memory returned when preparing a comment string is quite intricate: when the returned value is different than the passed value, then the caller is responsible to free the memory. This is quite subtle, and it's even easier to miss because the returned value is in fact a `const char *`. Adapt the function to always return either `NULL` or a newly allocated string. The function is called at most once per git-config(1), so it's not like this micro-optimization really matters. Thus, callers are now always responsible for freeing the value. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed May 6, 2024 at 10:55 UTC a78b462976d36304212f93b6d4a4c086a007362f
3 files changed +13 -16
builtin/config.c
+6 -5
@@ -44,7 +44,7 @@ static struct config_options config_options;
44 static int show_origin;
45 static int show_scope;
46 static int fixed_value;
47 -static const char *comment;
47 +static const char *comment_arg;
48
49 #define ACTION_GET (1<<0)
50 #define ACTION_GET_ALL (1<<1)
@@ -174,7 +174,7 @@ static struct option builtin_config_options[] = {
174 OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")),
175 OPT_BOOL(0, "show-scope", &show_scope, N_("show scope of config (worktree, local, global, system, command)")),
176 OPT_STRING(0, "default", &default_value, N_("value"), N_("with --get, use default value when missing entry")),
177 - OPT_STRING(0, "comment", &comment, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
177 + OPT_STRING(0, "comment", &comment_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
178 OPT_END(),
179 };
180
@@ -674,7 +674,7 @@ static char *default_user_config(void)
674 int cmd_config(int argc, const char **argv, const char *prefix)
675 {
676 int nongit = !startup_info->have_repository;
677 - char *value = NULL;
677 + char *value = NULL, *comment = NULL;
678 int flags = 0;
679 int ret = 0;
680 struct key_value_info default_kvi = KVI_INIT;
@@ -799,7 +799,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
799 usage_builtin_config();
800 }
801
802 - if (comment &&
802 + if (comment_arg &&
803 !(actions & (ACTION_ADD|ACTION_SET|ACTION_SET_ALL|ACTION_REPLACE_ALL))) {
804 error(_("--comment is only applicable to add/set/replace operations"));
805 usage_builtin_config();
@@ -841,7 +841,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
841 flags |= CONFIG_FLAGS_FIXED_VALUE;
842 }
843
844 - comment = git_config_prepare_comment_string(comment);
844 + comment = git_config_prepare_comment_string(comment_arg);
845
846 if (actions & PAGING_ACTIONS)
847 setup_auto_pager("config", 1);
@@ -993,6 +993,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
993 return get_colorbool(argv[0], argc == 2);
994 }
995
996 + free(comment);
997 free(value);
998 return ret;
999 }
config.c
+6 -10
@@ -3182,14 +3182,10 @@ void git_config_set(const char *key, const char *value)
3182 trace2_cmd_set_config(key, value);
3183 }
3184
3185 -/*
3186 - * The ownership rule is that the caller will own the string
3187 - * if it receives a piece of memory different from what it passed
3188 - * as the parameter.
3189 - */
3190 -const char *git_config_prepare_comment_string(const char *comment)
3185 +char *git_config_prepare_comment_string(const char *comment)
3186 {
3187 size_t leading_blanks;
3188 + char *prepared;
3189
3190 if (!comment)
3191 return NULL;
@@ -3210,13 +3206,13 @@ const char *git_config_prepare_comment_string(const char *comment)
3206
3207 leading_blanks = strspn(comment, " \t");
3208 if (leading_blanks && comment[leading_blanks] == '#')
3213 - ; /* use it as-is */
3209 + prepared = xstrdup(comment); /* use it as-is */
3210 else if (comment[0] == '#')
3215 - comment = xstrfmt(" %s", comment);
3211 + prepared = xstrfmt(" %s", comment);
3212 else
3217 - comment = xstrfmt(" # %s", comment);
3213 + prepared = xstrfmt(" # %s", comment);
3214
3219 - return comment;
3215 + return prepared;
3216 }
3217
3218 static void validate_comment_string(const char *comment)
config.h
+1 -1
@@ -338,7 +338,7 @@ void git_config_set_multivar(const char *, const char *, const char *, unsigned)
338 int repo_config_set_multivar_gently(struct repository *, const char *, const char *, const char *, unsigned);
339 int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, const char *, unsigned);
340
341 -const char *git_config_prepare_comment_string(const char *);
341 +char *git_config_prepare_comment_string(const char *);
342
343 /**
344 * takes four parameters: