revision: always store allocated strings in output encoding

The `git_log_output_encoding` variable can be set via the `--encoding=` option. When doing so, we conditionally either assign it to the passed value, or if the value is "none" we assign it the empty string. Depending on which of the both code paths we pick though, the variable may end up being assigned either an allocated string or a string constant. This is somewhat risky and may easily lead to bugs when a different code path may want to reassign a new value to it, freeing the previous value. We already to this when parsing the "i18n.logoutputencoding" config in `git_default_i18n_config()`. But because the config is typically parsed before we parse command line options this has been fine so far. Regardless of that, safeguard the code such that the variable always contains an allocated string. While at it, also free the old value in case there was any to plug a potential memory leak. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Jun 7, 2024 at 08:39 UTC 844d190677216c0754287f14d9474a55adf606a4
3 files changed +4 -1
revision.c
+2 -1
@@ -2650,10 +2650,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
2650 } else if (!strcmp(arg, "--invert-grep")) {
2651 revs->grep_filter.no_body_match = 1;
2652 } else if ((argcount = parse_long_opt("encoding", argv, &optarg))) {
2653 + free(git_log_output_encoding);
2654 if (strcmp(optarg, "none"))
2655 git_log_output_encoding = xstrdup(optarg);
2656 else
2656 - git_log_output_encoding = "";
2657 + git_log_output_encoding = xstrdup("");
2658 return argcount;
2659 } else if (!strcmp(arg, "--reverse")) {
2660 revs->reverse ^= 1;
t/t3900-i18n-commit.sh
+1
@@ -5,6 +5,7 @@
5
6 test_description='commit and log output encodings'
7
8 +TEST_PASSES_SANITIZE_LEAK=true
9 . ./test-lib.sh
10
11 compare_with () {
t/t3901-i18n-patch.sh
+1
@@ -8,6 +8,7 @@ test_description='i18n settings and format-patch | am pipe'
8 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
9 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
10
11 +TEST_PASSES_SANITIZE_LEAK=true
12 . ./test-lib.sh
13
14 check_encoding () {