[19/21] builtin/config: track "fixed value" option via flags only

Commit Message

Patrick Steinhardt May 10, 2024, 11:25 a.m. UTC

We track the "fixed value" option via two separate bits: once via the
global variable `fixed_value`, and once via the CONFIG_FLAGS_FIXED_VALUE
bit in `flags`. This is confusing and may easily lead to issues when one
is not aware that this is tracked via two separate mechanisms.

Refactor the code to use the flag exclusively. We already pass it to all
the require callsites anyway, except for `collect_config()`.

Signed-off-by: Patrick Steinhardt <[e-mail]>
---
 builtin/config.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Eric Sunshine May 11, 2024, 4:52 p.m. UTC | #1

On Sat, May 11, 2024 at 8:11 AM Patrick Steinhardt <[e-mail]> wrote:
> We track the "fixed value" option via two separate bits: once via the
> global variable `fixed_value`, and once via the CONFIG_FLAGS_FIXED_VALUE
> bit in `flags`. This is confusing and may easily lead to issues when one
> is not aware that this is tracked via two separate mechanisms.
>
> Refactor the code to use the flag exclusively. We already pass it to all
> the require callsites anyway, except for `collect_config()`.

s/require/required/

> Signed-off-by: Patrick Steinhardt <[e-mail]>

Patch

diff --git a/builtin/config.c b/builtin/config.c
index 8cedf67a4f..e6bc283751 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -124,7 +124,6 @@  struct config_display_options {
 
 static int use_key_regexp;
 static int do_all;
-static int fixed_value;
 
 #define TYPE_BOOL		1
 #define TYPE_INT		2
@@ -327,6 +326,7 @@  struct collect_config_data {
 	regex_t *regexp;
 	regex_t *key_regexp;
 	int do_not_match;
+	unsigned flags;
 };
 
 static int collect_config(const char *key_, const char *value_,
@@ -340,7 +340,8 @@  static int collect_config(const char *key_, const char *value_,
 		return 0;
 	if (use_key_regexp && regexec(data->key_regexp, key_, 0, NULL, 0))
 		return 0;
-	if (fixed_value && strcmp(data->value_pattern, (value_?value_:"")))
+	if ((data->flags & CONFIG_FLAGS_FIXED_VALUE) &&
+	    strcmp(data->value_pattern, (value_?value_:"")))
 		return 0;
 	if (data->regexp &&
 	    (data->do_not_match ^ !!regexec(data->regexp, (value_?value_:""), 0, NULL, 0)))
@@ -362,6 +363,7 @@  static int get_value(const struct config_location_options *opts,
 	struct collect_config_data data = {
 		.display_opts = display_opts,
 		.values = &values,
+		.flags = flags,
 	};
 	char *key = NULL;
 	int i;
@@ -1115,6 +1117,7 @@  static int cmd_config_actions(int argc, const char **argv, const char *prefix)
 	struct config_display_options display_opts = CONFIG_DISPLAY_OPTIONS_INIT;
 	const char *comment_arg = NULL;
 	int actions = 0;
+	unsigned flags = 0;
 	struct option opts[] = {
 		CONFIG_LOCATION_OPTIONS(location_opts),
 		OPT_GROUP(N_("Action")),
@@ -1137,13 +1140,12 @@  static int cmd_config_actions(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "default", &display_opts.default_value,
 			   N_("value"), N_("with --get, use default value when missing entry")),
 		OPT_STRING(0, "comment", &comment_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
-		OPT_BOOL(0, "fixed-value", &fixed_value, N_("use string equality when comparing values to 'value-pattern'")),
+		OPT_BIT(0, "fixed-value", &flags, N_("use string equality when comparing values to value pattern"), CONFIG_FLAGS_FIXED_VALUE),
 		OPT_BOOL(0, "includes", &location_opts.respect_includes_opt,
 			 N_("respect include directives on lookup")),
 		OPT_END(),
 	};
 	char *value = NULL, *comment = NULL;
-	int flags = 0;
 	int ret = 0;
 	struct key_value_info default_kvi = KVI_INIT;
 
@@ -1193,7 +1195,7 @@  static int cmd_config_actions(int argc, const char **argv, const char *prefix)
 	}
 
 	/* check usage of --fixed-value */
-	if (fixed_value) {
+	if (flags & CONFIG_FLAGS_FIXED_VALUE) {
 		int allowed_usage = 0;
 
 		switch (actions) {
@@ -1224,8 +1226,6 @@  static int cmd_config_actions(int argc, const char **argv, const char *prefix)
 			error(_("--fixed-value only applies with 'value-pattern'"));
 			exit(129);
 		}
-
-		flags |= CONFIG_FLAGS_FIXED_VALUE;
 	}
 
 	comment = git_config_prepare_comment_string(comment_arg);