Re: [RFC PATCH 4/5] strvec API users: fix leaks by using "STRVEC_INIT_NODUP"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 15.12.22 um 10:11 schrieb Ævar Arnfjörð Bjarmason:
> For these cases where all of the strings we're pushing to the "struct
> strvec" are fixed strings we can fix widespread memory leaks by
> skipping the xstrdup() on strvec_push().
>
> More in-tree users could benefit from this to save needless
> xstrdup()-ing, but for all of these we were munging the "v" member, so
> the subsequent strvec_clear() wouldn't free the memory.
>
> Now there's no need to free the individual elements, but we'll still
> need to free the container with the strvec_clear().
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
>  builtin/am.c                  | 2 +-
>  builtin/annotate.c            | 2 +-
>  builtin/stash.c               | 2 +-
>  t/t0023-crlf-am.sh            | 1 +
>  t/t4152-am-subjects.sh        | 2 ++
>  t/t4254-am-corrupt.sh         | 2 ++
>  t/t4256-am-format-flowed.sh   | 1 +
>  t/t4257-am-interactive.sh     | 2 ++
>  t/t5403-post-checkout-hook.sh | 1 +
>  9 files changed, 12 insertions(+), 3 deletions(-)

Looks nice and easy, though the test updates are a bit distracting.
They are all enabled by the change to builtin/am.c, right?

>
> diff --git a/builtin/am.c b/builtin/am.c
> index 30c9b3a9cd7..691ec1d152d 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1471,7 +1471,7 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
>  static int run_apply(const struct am_state *state, const char *index_file)
>  {
>  	struct strvec apply_paths = STRVEC_INIT;
> -	struct strvec apply_opts = STRVEC_INIT;
> +	struct strvec apply_opts = STRVEC_INIT_NODUP;
>  	struct apply_state apply_state;
>  	int res, opts_left;
>  	int force_apply = 0;
> diff --git a/builtin/annotate.c b/builtin/annotate.c
> index de58deadfc7..99d97c1a8c0 100644
> --- a/builtin/annotate.c
> +++ b/builtin/annotate.c
> @@ -9,7 +9,7 @@
>
>  int cmd_annotate(int argc UNUSED, const char **argv, const char *prefix)
>  {
> -	struct strvec args = STRVEC_INIT;
> +	struct strvec args = STRVEC_INIT_NODUP;
>  	int ret;
>
>  	strvec_pushl(&args, argv[0], "-c", NULL);
> diff --git a/builtin/stash.c b/builtin/stash.c
> index e504e22e0b9..b15dd2ebb3c 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -1823,7 +1823,7 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
>  {
>  	pid_t pid = getpid();
>  	const char *index_file;
> -	struct strvec args = STRVEC_INIT;
> +	struct strvec args = STRVEC_INIT_NODUP;
>  	parse_opt_subcommand_fn *fn = NULL;
>  	struct option options[] = {
>  		OPT_SUBCOMMAND("apply", &fn, apply_stash),
> diff --git a/t/t0023-crlf-am.sh b/t/t0023-crlf-am.sh
> index f9bbb91f64e..575805513a3 100755
> --- a/t/t0023-crlf-am.sh
> +++ b/t/t0023-crlf-am.sh
> @@ -2,6 +2,7 @@
>
>  test_description='Test am with auto.crlf'
>
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  cat >patchfile <<\EOF
> diff --git a/t/t4152-am-subjects.sh b/t/t4152-am-subjects.sh
> index 4c68245acad..9f2edba1f83 100755
> --- a/t/t4152-am-subjects.sh
> +++ b/t/t4152-am-subjects.sh
> @@ -1,6 +1,8 @@
>  #!/bin/sh
>
>  test_description='test subject preservation with format-patch | am'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  make_patches() {
> diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
> index 54be7da1611..45f1d4f95e5 100755
> --- a/t/t4254-am-corrupt.sh
> +++ b/t/t4254-am-corrupt.sh
> @@ -1,6 +1,8 @@
>  #!/bin/sh
>
>  test_description='git am with corrupt input'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  make_mbox_with_nul () {
> diff --git a/t/t4256-am-format-flowed.sh b/t/t4256-am-format-flowed.sh
> index 2369c4e17ad..1015273bc82 100755
> --- a/t/t4256-am-format-flowed.sh
> +++ b/t/t4256-am-format-flowed.sh
> @@ -2,6 +2,7 @@
>
>  test_description='test format=flowed support of git am'
>
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  test_expect_success 'setup' '
> diff --git a/t/t4257-am-interactive.sh b/t/t4257-am-interactive.sh
> index aed8f4de3d6..f26d7fd2dbd 100755
> --- a/t/t4257-am-interactive.sh
> +++ b/t/t4257-am-interactive.sh
> @@ -1,6 +1,8 @@
>  #!/bin/sh
>
>  test_description='am --interactive tests'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  test_expect_success 'set up patches to apply' '
> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> index 978f240cdac..cfaae547398 100755
> --- a/t/t5403-post-checkout-hook.sh
> +++ b/t/t5403-post-checkout-hook.sh
> @@ -7,6 +7,7 @@ test_description='Test the post-checkout hook.'
>  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 '




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux