config: let `config_store_data_clear()` handle `value_regex`

Instead of duplicating the logic for clearing up `value_regex`, let `config_store_data_clear()` handle that. When `regcomp()` fails, the current code does not call `regfree()`. Make sure we do the same by immediately invalidating `value_regex`. Some implementations are able to handle such an extra `regfree()`-call [1], but from the example in [2], we should not do so. (The language itself in [2] is not super-clear on this.) [1] https://www.redhat.com/archives/libvir-list/2013-September/msg00262.html [2] http://pubs.opengroup.org/onlinepubs/9699919799/functions/regcomp.html Researched-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Martin Ågren committed May 20, 2018 at 12:42 UTC 3b82542dff46339a585c7dde3a40bc543f6212f8
1 file changed +6 -11
config.c
+6 -11
@@ -2335,6 +2335,11 @@ struct config_store_data {
2335
2336 static void config_store_data_clear(struct config_store_data *store)
2337 {
2338 + if (store->value_regex != NULL &&
2339 + store->value_regex != CONFIG_REGEX_NONE) {
2340 + regfree(store->value_regex);
2341 + free(store->value_regex);
2342 + }
2343 free(store->parsed);
2344 free(store->seen);
2345 memset(store, 0, sizeof(*store));
@@ -2722,7 +2727,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2727 if (regcomp(store.value_regex, value_regex,
2728 REG_EXTENDED)) {
2729 error("invalid pattern: %s", value_regex);
2725 - free(store.value_regex);
2730 + FREE_AND_NULL(store.value_regex);
2731 ret = CONFIG_INVALID_PATTERN;
2732 goto out_free;
2733 }
@@ -2748,21 +2753,11 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2753 &store, &opts)) {
2754 error("invalid config file %s", config_filename);
2755 free(store.key);
2751 - if (store.value_regex != NULL &&
2752 - store.value_regex != CONFIG_REGEX_NONE) {
2753 - regfree(store.value_regex);
2754 - free(store.value_regex);
2755 - }
2756 ret = CONFIG_INVALID_FILE;
2757 goto out_free;
2758 }
2759
2760 free(store.key);
2761 - if (store.value_regex != NULL &&
2762 - store.value_regex != CONFIG_REGEX_NONE) {
2763 - regfree(store.value_regex);
2764 - free(store.value_regex);
2765 - }
2761
2762 /* if nothing to unset, or too many matches, error out */
2763 if ((store.seen_nr == 0 && value == NULL) ||