diff mbox series

[18/22] builtin/grep: fix leaking object context

Message ID 0722cb38ea9d6b158a07da6f31ad0fe83424cb2e.1724656120.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series Memory leak fixes (pt.6) | expand

Commit Message

Patrick Steinhardt Aug. 26, 2024, 7:22 a.m. UTC
Even when `get_oid_with_context()` fails it may have allocated some data
in tthe object context. But we do not release it in git-grep(1) when the
call fails, leading to a memory leak. Plug it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/grep.c                 | 1 +
 t/t6132-pathspec-exclude.sh    | 1 +
 t/t6135-pathspec-with-attrs.sh | 2 ++
 3 files changed, 4 insertions(+)

Comments

Junio C Hamano Sept. 4, 2024, 10:36 p.m. UTC | #1
Patrick Steinhardt <ps@pks.im> writes:

> Even when `get_oid_with_context()` fails it may have allocated some data
> in tthe object context. But we do not release it in git-grep(1) when the

"tthe" -> "the".

> call fails, leading to a memory leak. Plug it.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/grep.c                 | 1 +
>  t/t6132-pathspec-exclude.sh    | 1 +
>  t/t6135-pathspec-with-attrs.sh | 2 ++
>  3 files changed, 4 insertions(+)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index dfc3c3e8bd2..dda4582d646 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -1133,6 +1133,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  					 &oid, &oc)) {
>  			if (seen_dashdash)
>  				die(_("unable to resolve revision: %s"), arg);
> +			object_context_release(&oc);
>  			break;
>  		}

OK.  This is the "oh, this is not a revision argument" codepath.  It
is perfectly normal for get_oid_with_context() to fail here, and we
should make sure we clear the context variable.

Thanks.
diff mbox series

Patch

diff --git a/builtin/grep.c b/builtin/grep.c
index dfc3c3e8bd2..dda4582d646 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1133,6 +1133,7 @@  int cmd_grep(int argc, const char **argv, const char *prefix)
 					 &oid, &oc)) {
 			if (seen_dashdash)
 				die(_("unable to resolve revision: %s"), arg);
+			object_context_release(&oc);
 			break;
 		}
 
diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh
index 9fdafeb1e90..f31c09c056f 100755
--- a/t/t6132-pathspec-exclude.sh
+++ b/t/t6132-pathspec-exclude.sh
@@ -2,6 +2,7 @@ 
 
 test_description='test case exclude pathspec'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'setup' '
diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
index 120dcd74a51..794bc7daf05 100755
--- a/t/t6135-pathspec-with-attrs.sh
+++ b/t/t6135-pathspec-with-attrs.sh
@@ -1,6 +1,8 @@ 
 #!/bin/sh
 
 test_description='test labels in pathspecs'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'setup a tree' '