{"thread":{"id":"54202","subject":"[PATCH] add -p: fix memory leak","startedAt":"2020-09-07T15:08:19Z","lastAt":"2020-09-08T16:28:44Z","messageCount":2,"participants":["Phillip Wood via GitGitGadget","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"405122","messageId":"pull.729.git.1599491041170.gitgitgadget@gmail.com","threadId":"54202","inReplyTo":null,"subject":"[PATCH] add -p: fix memory leak","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-09-07T15:04:00Z","receivedAt":"2020-09-07T15:08:19Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nasan reports that the C version of `add -p` is not freeing all the\nmemory it allocates. Fix this by introducing a function to clear\n`struct add_p_state` and use it instead of freeing individual members.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n    add -p: fix memory leak\n    \n    It seems to be the season for add -p fixes. This patch fixes a memory\n    leak in the C version found with asan.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-729%2Fphillipwood%2Fwip%2Fadd-p-fix-memory-leak-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-729/phillipwood/wip/add-p-fix-memory-leak-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/729\n\n add-patch.c | 24 ++++++++++++++++--------\n 1 file changed, 16 insertions(+), 8 deletions(-)\n\ndiff --git a/add-patch.c b/add-patch.c\nindex 457b8c550e..2fcab983a6 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -266,6 +266,20 @@ struct add_p_state {\n \tconst char *revision;\n };\n \n+static void add_p_state_clear(struct add_p_state *s)\n+{\n+\tsize_t i;\n+\n+\tstrbuf_release(&s->answer);\n+\tstrbuf_release(&s->buf);\n+\tstrbuf_release(&s->plain);\n+\tstrbuf_release(&s->colored);\n+\tfor (i = 0; i < s->file_diff_nr; i++)\n+\t\tfree(s->file_diff[i].hunk);\n+\tfree(s->file_diff);\n+\tclear_add_i_state(&s->s);\n+}\n+\n static void err(struct add_p_state *s, const char *fmt, ...)\n {\n \tva_list args;\n@@ -1690,9 +1704,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \t     repo_refresh_and_write_index(r, REFRESH_QUIET, 0, 1,\n \t\t\t\t\t  NULL, NULL, NULL) < 0) ||\n \t    parse_diff(&s, ps) < 0) {\n-\t\tstrbuf_release(&s.plain);\n-\t\tstrbuf_release(&s.colored);\n-\t\tclear_add_i_state(&s.s);\n+\t\tadd_p_state_clear(&s);\n \t\treturn -1;\n \t}\n \n@@ -1707,10 +1719,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \telse if (binary_count == s.file_diff_nr)\n \t\tfprintf(stderr, _(\"Only binary files changed.\\n\"));\n \n-\tstrbuf_release(&s.answer);\n-\tstrbuf_release(&s.buf);\n-\tstrbuf_release(&s.plain);\n-\tstrbuf_release(&s.colored);\n-\tclear_add_i_state(&s.s);\n+\tadd_p_state_clear(&s);\n \treturn 0;\n }\n\nbase-commit: 3a238e539bcdfe3f9eb5010fd218640c1b499f7a\n-- \ngitgitgadget\n"},{"id":"405156","messageId":"nycvar.QRO.7.76.6.2009081409300.54@tvgsbejvaqbjf.bet","threadId":"54202","inReplyTo":"pull.729.git.1599491041170.gitgitgadget@gmail.com","subject":"Re: [PATCH] add -p: fix memory leak","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-09-08T12:09:50Z","receivedAt":"2020-09-08T16:28:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Mon, 7 Sep 2020, Phillip Wood via GitGitGadget wrote:\n\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>\n> asan reports that the C version of `add -p` is not freeing all the\n> memory it allocates. Fix this by introducing a function to clear\n> `struct add_p_state` and use it instead of freeing individual members.\n>\n> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nACK! And thanks for fixing bugs I introduced,\nDscho\n\n> ---\n>     add -p: fix memory leak\n>\n>     It seems to be the season for add -p fixes. This patch fixes a memory\n>     leak in the C version found with asan.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-729%2Fphillipwood%2Fwip%2Fadd-p-fix-memory-leak-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-729/phillipwood/wip/add-p-fix-memory-leak-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/729\n>\n>  add-patch.c | 24 ++++++++++++++++--------\n>  1 file changed, 16 insertions(+), 8 deletions(-)\n>\n> diff --git a/add-patch.c b/add-patch.c\n> index 457b8c550e..2fcab983a6 100644\n> --- a/add-patch.c\n> +++ b/add-patch.c\n> @@ -266,6 +266,20 @@ struct add_p_state {\n>  \tconst char *revision;\n>  };\n>\n> +static void add_p_state_clear(struct add_p_state *s)\n> +{\n> +\tsize_t i;\n> +\n> +\tstrbuf_release(&s->answer);\n> +\tstrbuf_release(&s->buf);\n> +\tstrbuf_release(&s->plain);\n> +\tstrbuf_release(&s->colored);\n> +\tfor (i = 0; i < s->file_diff_nr; i++)\n> +\t\tfree(s->file_diff[i].hunk);\n> +\tfree(s->file_diff);\n> +\tclear_add_i_state(&s->s);\n> +}\n> +\n>  static void err(struct add_p_state *s, const char *fmt, ...)\n>  {\n>  \tva_list args;\n> @@ -1690,9 +1704,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n>  \t     repo_refresh_and_write_index(r, REFRESH_QUIET, 0, 1,\n>  \t\t\t\t\t  NULL, NULL, NULL) < 0) ||\n>  \t    parse_diff(&s, ps) < 0) {\n> -\t\tstrbuf_release(&s.plain);\n> -\t\tstrbuf_release(&s.colored);\n> -\t\tclear_add_i_state(&s.s);\n> +\t\tadd_p_state_clear(&s);\n>  \t\treturn -1;\n>  \t}\n>\n> @@ -1707,10 +1719,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n>  \telse if (binary_count == s.file_diff_nr)\n>  \t\tfprintf(stderr, _(\"Only binary files changed.\\n\"));\n>\n> -\tstrbuf_release(&s.answer);\n> -\tstrbuf_release(&s.buf);\n> -\tstrbuf_release(&s.plain);\n> -\tstrbuf_release(&s.colored);\n> -\tclear_add_i_state(&s.s);\n> +\tadd_p_state_clear(&s);\n>  \treturn 0;\n>  }\n>\n> base-commit: 3a238e539bcdfe3f9eb5010fd218640c1b499f7a\n> --\n> gitgitgadget\n>\n"}]}