Merge 1a715ccbac into e326e52010

Merge branch 'rj/add-i-leak-fix'
Leakfix. * rj/add-i-leak-fix: add: plug a leak on interactive_add add-patch: plug a leak handling the '/' command add-interactive: plug a leak in get_untracked_files apply: plug a leak in apply_data
2024-05-08 21:26:08 +02:00 · 2024-04-25 23:49:18 +00:00 · 2024-04-25 10:34:24 -07:00 · 2024-04-25 10:34:23 -07:00 · 2024-04-22 16:27:43 -07:00 · 2024-04-22 16:27:42 -07:00
18 changed files with 29 additions and 9 deletions
--- a/add-interactive.c
+++ b/add-interactive.c
@ -865,6 +865,7 @@ static int get_untracked_files(struct repository *r,
 	}

 	strbuf_release(&buf);
+	dir_clear(&dir);
 	return 0;
 }

--- a/add-patch.c
+++ b/add-patch.c
@ -1646,6 +1646,7 @@ static int patch_update_file(struct add_p_state *s,
 				err(s, _("No hunk matches the given pattern"));
 				break;
 			}
+			regfree(&regex);
 			hunk_index = i;
 		} else if (s->answer.buf[0] == 's') {
 			size_t splittable_into = hunk->splittable_into;
--- a/apply.c
+++ b/apply.c
@ -3712,8 +3712,10 @@ static int apply_data(struct apply_state *state, struct patch *patch,
 			fprintf(stderr, _("Falling back to direct application...\n"));

 		/* Note: with --reject, apply_fragments() returns 0 */
-		if (patch->direct_to_threeway || apply_fragments(state, &image, patch) < 0)
+		if (patch->direct_to_threeway || apply_fragments(state, &image, patch) < 0) {
+			clear_image(&image);
 			return -1;
+		}
 	}
 	patch->result = image.buf;
 	patch->resultsize = image.len;
--- a/builtin/add.c
+++ b/builtin/add.c
@ -150,7 +150,7 @@ static int refresh(int verbose, const struct pathspec *pathspec)
 int interactive_add(const char **argv, const char *prefix, int patch)
 {
 	struct pathspec pathspec;
-	int unused;
+	int unused, ret;

 	if (!git_config_get_bool("add.interactive.usebuiltin", &unused))
 		warning(_("the add.interactive.useBuiltin setting has been removed!\n"
@ -163,9 +163,12 @@ int interactive_add(const char **argv, const char *prefix, int patch)
 		       prefix, argv);

 	if (patch)
-		return !!run_add_p(the_repository, ADD_P_ADD, NULL, &pathspec);
+		ret = !!run_add_p(the_repository, ADD_P_ADD, NULL, &pathspec);
 	else
-		return !!run_add_i(the_repository, &pathspec);
+		ret = !!run_add_i(the_repository, &pathspec);
+
+	clear_pathspec(&pathspec);
+	return ret;
 }

 static int edit_patch(int argc, const char **argv, const char *prefix)
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@ -18,7 +18,7 @@ mode_ok () {
 }

 is_available () {
-	merge_tool_path=$(translate_merge_tool_path "$1") &&
+	merge_tool_path=$(get_merge_tool_path "$1") &&
 	type "$merge_tool_path" >/dev/null 2>&1
 }

--- a/mem-pool.c
+++ b/mem-pool.c
@ -4,6 +4,7 @@

 #include "git-compat-util.h"
 #include "mem-pool.h"
+#include "gettext.h"

 #define BLOCK_GROWTH_SIZE (1024 * 1024 - sizeof(struct mp_block))

@ -122,7 +123,7 @@ static char *mem_pool_strvfmt(struct mem_pool *pool, const char *fmt,
 	len = vsnprintf(next_free, available, fmt, cp);
 	va_end(cp);
 	if (len < 0)
-		BUG("your vsnprintf is broken (returned %d)", len);
+		die(_("unable to format message: %s"), fmt);

 	size = st_add(len, 1); /* 1 for NUL */
 	ret = mem_pool_alloc(pool, size);
--- a/strbuf.c
+++ b/strbuf.c
@ -277,7 +277,7 @@ void strbuf_vinsertf(struct strbuf *sb, size_t pos, const char *fmt, va_list ap)
 	len = vsnprintf(sb->buf + sb->len, 0, fmt, cp);
 	va_end(cp);
 	if (len < 0)
-		BUG("your vsnprintf is broken (returned %d)", len);
+		die(_("unable to format message: %s"), fmt);
 	if (!len)
 		return; /* nothing to do */
 	if (unsigned_add_overflows(sb->len, len))
@ -404,7 +404,7 @@ void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)
 	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, cp);
 	va_end(cp);
 	if (len < 0)
-		BUG("your vsnprintf is broken (returned %d)", len);
+		die(_("unable to format message: %s"), fmt);
 	if (len > strbuf_avail(sb)) {
 		strbuf_grow(sb, len);
 		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
--- a/t/t2016-checkout-patch.sh
+++ b/t/t2016-checkout-patch.sh
@ -2,6 +2,7 @@

 test_description='git checkout --patch'

+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-patch-mode.sh

 test_expect_success 'setup' '
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@ -4,6 +4,7 @@ test_description='add -i basic tests'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME

+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh

--- a/t/t4103-apply-binary.sh
+++ b/t/t4103-apply-binary.sh
@ -9,6 +9,7 @@ test_description='git apply handling binary patches
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME

+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh

 test_expect_success 'setup' '
--- a/t/t4104-apply-boundary.sh
+++ b/t/t4104-apply-boundary.sh
@ -5,6 +5,7 @@

 test_description='git apply boundary tests'

+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh

 L="c d e f g h i j k l m n o p q r s t u v w x"
--- a/t/t4113-apply-ending.sh
+++ b/t/t4113-apply-ending.sh
@ -6,6 +6,7 @@
 test_description='git apply trying to add an ending line.

 '
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh

 # setup
--- a/t/t4117-apply-reject.sh
+++ b/t/t4117-apply-reject.sh
@ -7,6 +7,7 @@ test_description='git apply with rejects

 '

+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh

 test_expect_success setup '
--- a/t/t4123-apply-shrink.sh
+++ b/t/t4123-apply-shrink.sh
@ -2,6 +2,7 @@

 test_description='apply a patch that is larger than the preimage'

+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh

 cat >F  <<\EOF
--- a/t/t4252-am-options.sh
+++ b/t/t4252-am-options.sh
@ -1,6 +1,8 @@
 #!/bin/sh

 test_description='git am with options and not losing them'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh

 tm="$TEST_DIRECTORY/t4252"
--- a/t/t4258-am-quoted-cr.sh
+++ b/t/t4258-am-quoted-cr.sh
@ -2,6 +2,7 @@

 test_description='test am --quoted-cr=<action>'

+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh

 DATA="$TEST_DIRECTORY/t4258"
--- a/t/t7514-commit-patch.sh
+++ b/t/t7514-commit-patch.sh
@ -1,6 +1,8 @@
 #!/bin/sh

 test_description='hunk edit with "commit -p -m"'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh

 test_expect_success 'setup (initial)' '
--- a/wrapper.c
+++ b/wrapper.c
@ -670,7 +670,7 @@ int xsnprintf(char *dst, size_t max, const char *fmt, ...)
 	va_end(ap);

 	if (len < 0)
-		BUG("your snprintf is broken");
+		die(_("unable to format message: %s"), fmt);
 	if (len >= max)
 		BUG("attempt to snprintf into too-small buffer");
 	return len;
Author	SHA1	Message	Date
Michael Schindler	1f44882efb	Merge `1a715ccbac` into `e326e52010`	2024-04-25 23:49:18 +00:00
Junio C Hamano	e326e52010	Merge branch 'rj/add-i-leak-fix' Leakfix. * rj/add-i-leak-fix: add: plug a leak on interactive_add add-patch: plug a leak handling the '/' command add-interactive: plug a leak in get_untracked_files apply: plug a leak in apply_data	2024-04-25 10:34:24 -07:00
Junio C Hamano	c9d1ee7cdf	Merge branch 'rs/vsnprintf-failure-is-not-a-bug' Demote a BUG() to an die() when the failure from vsnprintf() may not be due to a programmer error. * rs/vsnprintf-failure-is-not-a-bug: don't report vsnprintf(3) error as bug	2024-04-25 10:34:23 -07:00
Rubén Justo	16727404c4	add: plug a leak on interactive_add Plug a leak we have since `5a76aff1a6` (add: convert to use parse_pathspec, 2013-07-14). This leak can be triggered with: $ git add -p anything Fixing this leak allows us to mark as leak-free the following tests: + t3701-add-interactive.sh + t7514-commit-patch.sh Mark them with "TEST_PASSES_SANITIZE_LEAK=true" to notice and fix promply any new leak that may be introduced and triggered by them in the future. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	2024-04-22 16:27:43 -07:00
Rubén Justo	ec9b74b18e	add-patch: plug a leak handling the '/' command Plug a leak we have since `d6cf873340` (built-in add -p: implement the '/' ("search regex") command, 2019-12-13). This leak can be triggered with: $ printf "A\n\nB\n" >file $ git add file && git commit -m file $ printf "AA\n\nBB\n" >file $ printf "s\n/ .\n" >lines $ git add -p <lines Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	2024-04-22 16:27:42 -07:00
Rubén Justo	5861aa84a7	add-interactive: plug a leak in get_untracked_files Plug a leak we have since `ab1e1cccaf` (built-in add -i: re-implement `add-untracked` in C, 2019-11-29). This leak can be triggered with: $ echo a \| git add -i As a curiosity, we have a somewhat similar function in builtin/stash.c, which correctly frees the memory. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	2024-04-22 16:27:42 -07:00
Rubén Justo	71c7916053	apply: plug a leak in apply_data We have an execution path in apply_data that leaks the local struct image. Plug it. This leak can be triggered with: $ echo foo >file $ git add file && git commit -m file $ echo bar >file $ git diff file >diff $ sed s/foo/frotz/ <diff >baddiff $ git apply --cached <baddiff Fixing this leak allows us to mark as leak-free the following tests: + t2016-checkout-patch.sh + t4103-apply-binary.sh + t4104-apply-boundary.sh + t4113-apply-ending.sh + t4117-apply-reject.sh + t4123-apply-shrink.sh + t4252-am-options.sh + t4258-am-quoted-cr.sh Mark them with "TEST_PASSES_SANITIZE_LEAK=true" to notice and fix promply any new leak that may be introduced and triggered by them in the future. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	2024-04-22 16:27:42 -07:00
René Scharfe	0283cd5161	don't report vsnprintf(3) error as bug strbuf_addf() has been reporting a negative return value of vsnprintf(3) as a bug since `f141bd804d` (Handle broken vsnprintf implementations in strbuf, 2007-11-13). Other functions copied that behavior: `7b03c89ebd` (add xsnprintf helper function, 2015-09-24) `5ef264dbdb` (strbuf.c: add `strbuf_insertf()` and `strbuf_vinsertf()`, 2019-02-25) `8d25663d70` (mem-pool: add mem_pool_strfmt(), 2024-02-25) However, vsnprintf(3) can legitimately return a negative value if the formatted output would be longer than INT_MAX. Stop accusing it of being broken and just report the fact that formatting failed. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	2024-04-21 12:27:07 -07:00
Michael Schindler	1a715ccbac	use get_merge_tool_path() also in is_available() to honor settings fix the is_available test used in git mergetool --tool-help or git difftool --tool-help or to check the list of tools available when no tool is configured/given with --tool symtoms: the actual tool running run_merge_tool () considers the difftool. "$merge_tool".path and mergetool."$merge_tool".path and if configured honors these. See get_merge_tool_path () in git-mergetool--lib.sh If not set use fallback: translate_merge_tool_path "$merge_tool". The is_available () just uses translate_merge_tool_path "$merge_tool". repo 1: Configure an invalid path in mergetool."$merge_tool".path for a tool of your choice. You will be informed that the tool is available, but when trying to use it will not be found because the invalid configured path is used. repo2: Install a tool of your choice on a nonstandard place (e.g. rename the program) and configure mergetool."$merge_tool".path for this tool. You will be informed that the tool is not available (because not found on standard place), but when trying to run it will work. This fix will make the information consistent by using get_merge_tool_path() also in is_available() Signed-off-by: Michael Schindler <michael@compressconsult.com>	2021-06-07 22:01:53 +02:00