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>

diff --git a/revision.c b/revision.c
index 7ddf0f151a381077c7e30d2a9835c4ec64947006..2ee6886078cf1d663877cac57859f72f06f165d9 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -2650,10 +2650,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
        } else if (!strcmp(arg, "--invert-grep")) {
                revs->grep_filter.no_body_match = 1;
        } else if ((argcount = parse_long_opt("encoding", argv, &optarg))) {
+               free(git_log_output_encoding);
                if (strcmp(optarg, "none"))
                        git_log_output_encoding = xstrdup(optarg);
                else
-                       git_log_output_encoding = "";
+                       git_log_output_encoding = xstrdup("");
                return argcount;
        } else if (!strcmp(arg, "--reverse")) {
                revs->reverse ^= 1;

diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
index f27d09cfd9439ae31ec707fe738356bca88064f6..db7b403bc1541d8eed62eb65f21ce5ba262d8128 100755 (executable)
--- a/t/t3900-i18n-commit.sh
+++ b/t/t3900-i18n-commit.sh
@@ -5,6 +5,7 @@
 
 test_description='commit and log output encodings'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 compare_with () {

diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
index 4b37f78829819ff023db64b026fc6b8ea4061deb..5f0b9afc3faa7d09013c8eaa6958e7c6786f1095 100755 (executable)
--- a/t/t3901-i18n-patch.sh
+++ b/t/t3901-i18n-patch.sh
@@ -8,6 +8,7 @@ test_description='i18n settings and format-patch | am pipe'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 check_encoding () {