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.
> ---
> 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
