Re: [PATCH v2 1/1] maintenance: fix two memory leaks
- From
Derrick Stolee <stolee@gmail.com>
- Date
- May 11, 2021, 15:13 UTC
- Message-ID
- <01dccb51-cb2e-ea6f-a05c-d76bb7ed725f@gmail.com>
- In-Reply-To
- <20210510195909.621534-2-lenaic@lhuard.fr>
On 5/10/2021 3:59 PM, Lénaïc Huard wrote:
> Fixes two memory leaks when running `git maintenance start` or `git > maintenance stop` in `update_background_schedule`:
Thanks for finding these leaks.
Show 36 quoted lines
> ---
> builtin/gc.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index ef7226d7bc..484fe983d3 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1924,6 +1924,7 @@ static int crontab_update_schedule(int run_maintenance, int fd, const char *cmd)
> else if (!in_old_region)
> fprintf(cron_in, "%s\n", line.buf);
> }
> + strbuf_release(&line);
>
> if (run_maintenance) {
> struct strbuf line_format = STRBUF_INIT;
> @@ -1986,8 +1987,10 @@ static int update_background_schedule(int enable)
> cmd = sep + 1;
> }
>
> - if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0)
> - return error(_("another process is scheduling background maintenance"));
> + if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) {
> + result = error(_("another process is scheduling background maintenance"));
> + goto cleanup;
> + }
>
> if (!strcmp(scheduler, "launchctl"))
> result = launchctl_update_schedule(enable, get_lock_file_fd(&lk), cmd);
> @@ -1999,6 +2002,9 @@ static int update_background_schedule(int enable)
> die("unknown background scheduler: %s", scheduler);
>
> rollback_lock_file(&lk);
> +
> +cleanup:
> + free(lock_path);And I agree that this version looks good.
Thanks, -Stolee